3
\$\begingroup\$

Recently I published a post in which three people made pretty good suggestions. I've learnt a lot since then thanks to them!

From among many things, they encouraged me to use classes. And that's cool because it is the first time I do object oriented programming... Even though the use I gave it was very basic.

Once again, I'd like to receive reviews of my new code. It's uploaded in GitHub, in case someone wants to check it out.

I'm building this dictionary on python, using data.json.

Any suggestions are warmly welcome! So, how can I improme my code?

import json, time, re
from difflib import get_close_matches

class fuzzydict(object):

    def __init__(self, json_file):
        self.file = json_file
        self.data = json.load(open(json_file, 'r+'))

    def find_word(self, word):
        for version in [word.lower(), word.capitalize(), word.title(), word.upper()]:
            if version in self.data:
                return version
        simils = get_close_matches(word, self.data, 1, 0.7)
        if len(simils) > 0:
            return simils[0]
        else:
            return None

    def output_word(self, word):
        # check if 'keyword' has no values (definitions)
        if not self.data[word]:
            print('"%s" is yet to be defined.' % word)
        # print in a cool format
        else:
            print('· ' + word + ':')
            # re.split('--|;', self.data[word]) in case the
            # definitions were not given inside a list
            for index, definition in enumerate(self.data[word]):
                print(str(index + 1) + '.', definition)

    def input_word(self, word, definition):
        operation = 0
        # this prevents the user from adding an already existing definition
        if word in self.data and definition not in self.data[word]:
            self.data[word] += [definition]
            operation = 1
        # in case it's a new word
        elif word not in self.data:
            self.data.update({word: [definition]})
            operation = 1
        # updates the file when necessary
        if operation:
            with open(self.file, 'w') as file:
                json.dump(self.data, file)
                return '\nDone!'
        return '\nOUPS! Apparently the definition you attempted to add already exists.'

    def remove_word(self, word):
        if word != None:
            self.data.pop(word, None)
            with open(self.file, 'w') as file:
                json.dump(self.data, file)
                return '\nDone!'
        else:
            return "\nHmm... how can you remove something that doesn't exist? Huh!"

    def remove_def(self, word, index):
        for i, definition in enumerate(self.data[word]):
            if i == int(index) - 1:
                self.data[word].remove(definition)
                with open(self.file, 'w') as file:
                    json.dump(self.data, file)
                    return '\nDone!'
        return "\nHmm... how can you remove something that doesn't exist? Huh!"

# new object
mydict = fuzzydict('data.json')
# can either access the data through 'archives' or 'mydict.data'

while True:
    menu = input('\n{\n\n'
                 '0. Quit\n'
                 '1. Search\n'
                 '2. Add word or definition\n'
                 '3. Remove word\n'
                 '4. Remove definition\n\n'
                 '} '
                 'What would you like to do? ')

    # '0' to exit
    if menu == '0':
        break

    # '1' to look up a word
    elif menu == '1':
        search = input('\nType in a word: ')
        if mydict.find_word(search) == None:
            print('"' + search + '"' + " isn't available at the moment.")
            yes = input('Would you like to add "' + search + '" to the dictionary? ')
            if yes.lower() == 'yes':
                meaning = input('Type in the meaning of ' + search + ', please: ')
                while meaning == '':
                    meaning = input('Type in a valid definition, please:')
                print(mydict.input_word(search, meaning))
        else:
            mydict.output_word(mydict.find_word(search))

    # '2' to add or remove a new word or definition
    elif menu == '2':
        print('~ You are now editing the dictionary ~')
        new_word = input('\tWord: ')
        new_def = input('\tDefinition: ')
        if mydict.find_word(new_word) == None:
            print(mydict.input_word(new_word, new_def))
        else:
            print(mydict.input_word(mydict.find_word(new_word), new_def))

    # '3' to remove an existing word
    elif menu == '3':
        print('~ You are now editing the dictionary ~')
        rm_word = input('\tType in the word you want to remove from the dictionary: ')
        print(mydict.remove_word(mydict.find_word(rm_word)))

    # '4' to remove an existing definition using its ID
    elif menu == '4':
        print('~ You are now editing the dictionary ~')
        obj_word = input('\tWord: ')
        mydict.output_word(obj_word)
        id_def = input("\nWhich definition do you want to remove? ")
        print(mydict.remove_def(obj_word, id_def))

    # 5 seconds delay, good for UX
    print('\nLoading...')
    time.sleep(5)
\$\endgroup\$
1
\$\begingroup\$

User experience:

  1. Quit option usually is the last one in the list.
  2. Some words definitions are so long that I had to scroll horizontally.
  3. Going back to main menu after few seconds pass seems not very user-friendly. When there are many definitions you just don't have enough time to read all of them. I think it's better to go back to menu if some button is pressed.
  4. Also, while I'm shown a list of definitions there is this Loading... printed. It's a bit confusing. I was expecting more definitions to be printed out. But in fact there is no loading at all. Just waiting for some time.
  5. Printing out similar words seems like a good idea, but sometimes it gives strange results. For example, if I wanted to look for loan but instead typed loqn, it will give me long.
  6. If I made a mistake and typed word incorrectly when I wanted to delete it, the program wouldn't warn me and just delete the most similar word. There should be a warning about what word exactly is going to be deleted.
  7. Also, what if I change my mind about deleting word? I think there should be a way to go back to main menu.
  8. If I search for a word that doesn't exist, program asks if I want to add it to the dictionary. I typed y and expected it to be saved. But it didn't because you are checking only for yes.

PEP 8:

  1. Imports should be on separate lines. Also, re is never used.

  2. Class definition should be surrounded by 2 blank lines.

  3. Class names are usually in CamelCase.

  4. if len(simils) > 0: should be replaced by simply if simils:. Also, why not rename it to similar_words?

output_word:

  1. It's possible to avoid unnecessary nesting here like this:

    if not self.data[word]:
        print(f'"{word}" is yet to be defined.')
        return
    
    print('· ' + word + ':')
    for index, definition in enumerate(self.data[word]):
        print(str(index + 1) + '.', definition)
    
  2. Note the f-string above. It's a new feature of Python 3.6.

input_word:

  1. There is no need for a flag-variable operation. It is possible to avoid it here after some refactoring.

  2. Don't return strings like Done!. In your case you just want to print them and return nothing.

  3. It is possible to significantly reduce the code using get and setdefault methods like this:

    if definition in self.data.get(word, []):
        print('\nOUPS! Apparently the definition you attempted to add '
              'already exists.')
        return
    
    self.data.setdefault(word, []).append(definition)
    with open(self.file, 'w') as file:
        json.dump(self.data, file)
    print('\nDone!')
    

remove_word

  1. Same as in output_word. First, check if word is None then without nesting with else remove the word from the dictionary.

remove_def:

  1. There is no need to iterate over definitions here. Just remove it by index and catch exceptions:

    try:
        self.data[word].pop(index - 1)
    except IndexError:
        print("\nHmm... how can you remove something that doesn't exist? "
              "Huh!")
        return
    with open(self.file, 'w') as file:
        json.dump(self.data, file)
    print("\nDone!")
    

Other notes:

  1. Everything that you put outside of a class should be put in functions. And use
    if __name__ == '__main__':

  2. Now you are opening, recording, closing your json file for every operation. Consider applying changes locally, and recording everything at once only in the very end, when you finish working with your dictionary.

\$\endgroup\$
  • 1
    \$\begingroup\$ That was knowledge enriching! Thank you so much, Georgy!! \$\endgroup\$ – user157281 Jan 10 '18 at 10:00
2
\$\begingroup\$

You are particular about supporting various forms of the input word. I would suggest that you create a normalized key dictionary that maps lowercased versions of the key to a list of forms stored in the main dict.

That is:

def find_word(self, word):
    lcword = word.lower()
    stored_cases = self.get_keys[lcword]  # Map 'cat' -> ['Cat', 'CAT', 'cat']

    for key in stored_cases:
        yield key

I would also suggest that you utilize None instead of your 'N0t_F0uNd' string. This case is pretty much why it exists - to express the idea that nothing is available. Except for the next suggestion...

I would also suggest that you write your code expecting find_word to return multiple values. Actually, to generate multiple values as an iterator. So your code wouldn't check for a sentinel value meaning "I got nothing." Instead, it would iterate over all possible values, and possibly special-case the empty sequence:

for w in my_dict.find_word('cat'):
    ... etc ...

Finally, I would suggest that instead of dropping into insert mode when a match is not found, you print a message saying "Select option 2 to add it" or something. For extra points, you can remember the last searched word and make that the default during add/remove operations.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy