Ya vse. #1
Ya vse. #1
Conversation
| class JsonWriter: | ||
| @staticmethod | ||
| def write_json(name, jsn): | ||
| with open(name, 'w') as file: # serialization |
There was a problem hiding this comment.
This class is very good in terms of SRP, but this comment is redundant - the code is pretty self-documenting.
|
|
||
| class CheckerForMeaning: | ||
| @staticmethod | ||
| def find_trash(clean_line, inf_dict): |
There was a problem hiding this comment.
The idea to mutate the data passed as an argument is as bad as mutating the internal state. That means, instead of doing
def find_trash(clean_line, inf_dict):
# ...
inf_dict['orphan_tokens'].append(i)
# ...a much better strategy could be
def find_trash(clean_line):
# ...
return clean_tokens, orphan_tokens
# somewhere higher the call stack
clean_tokens, orphan_tokens = find_trash
response["orphan_tokens"] = orphan_tokensDon't ever mutate the data passes to your methods as an argument.
| for i in clean_tokens: # check for meaning | ||
| if nltk.wsd.lesk(tokens, i) is None: | ||
| inf_dict['orphan_tokens'].append(i) | ||
| return inf_dict |
There was a problem hiding this comment.
If you absolutely have to mutate the data sent you as an argument, don't return this very data from your method - your method should either have an "out" parameter to mutate, or a return value, not both. Having both, you confuse a reader of your method.
|
|
||
| result = {'records': []} | ||
|
|
||
| for i in data: # tweet handling |
There was a problem hiding this comment.
i could be line for better readability.
| def __init__(self, line): | ||
| self.line = line | ||
|
|
||
| def explore(self): |
There was a problem hiding this comment.
The responsibilities distribution between lines is done well enough.
There was a problem hiding this comment.
And the idea to get parts of the response data and merge them in a single place is also a very good way to go. Just remove any mutations of the input data in your methods (see my comment above).
| def find_urls(clean_line, raw_line, inf_dict): | ||
| """ find URL """ | ||
| urls = re.findall('http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+', raw_line) | ||
| funk_for_clean = RemoverUnwantedWords.remove_words |
There was a problem hiding this comment.
- Unnecessary indirection.
- English:
funcinstead offunk.
2 participants
No description provided.