PCC01 ryanh153 #571
PCC01 ryanh153 #571
Conversation
|
Difficulty level (1-10): [3] |
| with open(DICTIONARY,'r') as dicFile: | ||
| dictionary = dicFile.readlines() | ||
|
|
||
| dictionary = [word.strip() for word in dictionary] |
There was a problem hiding this comment.
just a minor issue but the name "dictionary" implies to the reader that the content of that variable is a Python dictionary, but it is a list. This is of course in this exercise a little bit hard to avoid, but in general, its a good way to have the type of a variable in the name, for example a list_of_words is a Python list holding words, which the reader expects then to be strings.
| """Calculate the value of the word entered into function | ||
| using imported constant mapping LETTER_SCORES""" | ||
| def calc_word_value(word): | ||
| return sum([LETTER_SCORES[char] for char in word.upper() if char in LETTER_SCORES.keys()]) |
There was a problem hiding this comment.
- in Python
char in dict.keys()is identical tochar in dictso you can ommit the call tokeys() - Because you don't need the list for further processing, better to use a generator inside the sum, so there is no need to calculate first the score of all letters. Of course here the memory consumption is rather low and limited to the maximum length of a given word, but nevertheless, good style to avoid lists when you dont explitly need them
| """Calculate the word with the max value, can receive a list | ||
| of words as arg, if none provided uses default DICTIONARY""" | ||
| def max_word_value(words=None): | ||
| if words == None: |
There was a problem hiding this comment.
this is correct, but not the Pythonic way to do things. You have to better options:
- Use the Python special "is":
if words is Nonewhich avoids the equal operator - Even better, use just
if words. This is a shorthand evaluation that isTrueeither for empty lists orNonesoif words:is a good way to prove for empty or nonexisting variables
| return words[scores.index(max(scores))] | ||
|
|
||
| if __name__ == "__main__": | ||
| pass # run unittests to validate |
There was a problem hiding this comment.
you could solve this little comment by replacing pass with unittest.main()
This file is not part of your solution and rather big. You don't have to commit files of the original challenge :)
Again, you don't have to add template files to your solution, just your solution :)
This file is part of the challenge and not your code so I deleted it from the PR
Tests are part of the challenge and not your contribution so I deleted them from the pR :)
ATTENTION: before clicking "Create Pull Request" please submit some meta data, thanks!
Difficulty level (1-10): []
Estimated time spent (hours): []
Completed (yes/no): []
I stretched my coding skills (if yes what did you learn?): []
Other feedback (what can we improve?): []