5
\$\begingroup\$

I'm currently building an interactive dictionary. I'm using a sample JSON file (i.e. data.json) as a template. But that's not it. My aim is to build an app where I can store the knowledge that I acquire during my learning process.

And I'd like you to take a look at my code to see if there are areas where I can improve the performance of my program.

Code

import json
from difflib import get_close_matches

data = json.load(open('data.json', 'r'))

def newdict(word, definition):
    data.update({word: [definition]})
    with open('data.json', 'w') as file:
        json.dump(data, file)


def tr(word):
    gcm = get_close_matches(word, data, 1, 0.8)
    if word in data:
        print('· ' + word + ':')
        for index, definition in enumerate(data[word]):
            print(str(index+1) + '.', definition)
    elif word.title() in data:
        print('· ' + word.title() + ':')
        for index, definition in enumerate(data[word.title()]):
            print(str(index + 1) + '.', definition)
    elif word.upper() in data:
        print('· ' + word.upper() + ':')
        for index, definition in enumerate(data[word.upper()]):
            print(str(index + 1) + '.', definition)
    elif len(gcm) > 0:
        print('· ' + gcm[0].capitalize() + ':')
        for index, definition in enumerate(data[gcm[0]]):
                print(str(index+1) + '.', definition)
    else:
        print("'%s' definition isn't available at the moment." % word.capitalize())
        x = input('Would you like to add "%s" to the dictionary? ' % word.capitalize())
        x = x.lower()
        if x == 'yes':
            y = input('Type in the meaning of %s: ' % word)
            newdict(word, y)
        else:
            print('Ok, as you wish!')

w = input('Enter a word: ')
w = w.lower()
while w != '0':
    tr(w)
    w = input('\nEnter a word: ')
    w = w.lower()

Is there room for improvement?

\$\endgroup\$
5
\$\begingroup\$

The biggest thing that jumps out at me is that your project isn't in the form of a class, it's a function. This is, while not quite discouraged, certainly non-advantageous. Classes make things more portable. If I were building this, I'd structure it something like this:

class fuzzydict:
  """Fuzztdict: A dictionary with inexact lookups
       #Function definitions, etc go here.
  """
  def __init__(self):
    #Create the data file, initialize all your other variables.
  def __str__(self):
    #This one isn't necessary, but it would allow you to customize what would happen when you call print on a fuzzydict.
  def find_word(self,word):
    #Find a word in the dictionary, or report that it (probably) doesn't exist.
  def add_word(self,word,definition):
    #Add a word to the dictionary. 

Then, you'd instantiate it with newDict = fuzzydict() and you'd be good to go. You can use stuff like newDict.find_word('apple'), which is a much more intuitive interface than your while loop.

A few other things about your code:

  1. You're checking various versions of your data in the JSON. Do you intend to eventually be able to input data yourself? Then I'd suggest formatting your entries into all UPPER or all lower case, then including how they were originally inputted as an attribute for that entry. This'll eliminate the multiple checks.

  2. Error handling. The difference between a good program and a truly epic program is how gracefully it can recover from something it didn't expect.

  3. As stated in other answers, document. Document, document, document. Even when it's just a particularly interesting way of doing something, make your code as descriptive as possible. Future you will thank you, as well as anybody else who tries to decipher the finished project. Remember docstrings, they're a gift from whatever deities the creators of Python worship.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot, I'm learning a lot by all of your contributions! Much love <3 \$\endgroup\$ – user157281 Jan 5 '18 at 11:12
3
\$\begingroup\$

gcm = get_close_matches(word, data, 1, 0.8) should be moved to right before the elif block that uses it; there's no point calling a function if you don't use it. Also, since there's nothing after all the elif/else blocks, you can just return at the end of each block, and then you can change the elif's to if's (with returns at the end of each block, you can't get to a block unless the previous conditions have been false). Building on Julian's answer, you can do:

for version in [word, word.title,word.upper()]:
      if version in data:
          print_word(data,version)
          return  #Note that, as Julien says, good practice is to return the string rather than printing it)
gcm = get_close_matches(word, data, 1, 0.8)
if len(gcm) > 0:
    print_word(data,gcm[0])

For your while loop at the end, you can eliminate repeating w = input('Enter a word: ') and w = w.lower() by doing the following:

while True:
    w = input('Enter a word: ')
    if w == '0':
        break
    w = w.lower()
    tr(w)

Also, if you're using '0' as the quit key, then you should tell the user that somewhere.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot, I'm learning a lot by all of your contributions! Much love <3 \$\endgroup\$ – user157281 Jan 5 '18 at 11:12
2
\$\begingroup\$

very nice thing to have a Github repo!

I'd say first thing that pop to my eyes is the naming of variables and function. tr is not an acceptable name, it does not convey anything meaningful. And it would also help to have some documentation, docstring or the like to help decipher your code.

gcm is also not a good name, I guess it stand for get_close_matches because the function is just after but it's pretty hard to tell anywhere else. Same for x and w at the end, they could be named like input_for_dictionnary and user_input (those are not great names either but it's a start).

Secondly I'd cut every if / else if code into function to help clarity, especially because you repeat a lot of things. See

if word in data:
    print('· ' + word + ':')
    for index, definition in enumerate(data[word]):
        print(str(index+1) + '.', definition)
elif word.title() in data:
    print('· ' + word.title() + ':')
    for index, definition in enumerate(data[word.title()]):
        print(str(index + 1) + '.', definition)

Let's say you have a function like (that would need a better name and a docstring as well):

 def print_word(data, text):
      print('· ' + text + ':')
      for index, definition in enumerate(data[text]):
        print(str(index + 1) + '.', definition)

Then you could rewrite your precedent condition with:

if word in data:
    print_word(data, word)
elif word.title() in data:
    print_word(data, word.title())

Better yet would be to have the function return a string instead of printing, but it is left as an exercise :)

Good job otherwise, nice project.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot, I'm learning a lot by all of your contributions! Much love <3 \$\endgroup\$ – user157281 Jan 5 '18 at 11:12

Your Answer

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