4
\$\begingroup\$

Post a cursory read of this I implemented a simple dictionary and an interface to assign, replace, look up, and redefine its terms to apply the concepts.

It's simple, but I'd still like to know any way I could make this cleaner and more pythonic.

def print_dict(dictionary):
  for key in dictionary:
    print("{} : {}".format(key, dictionary[key]))

def display_menu():
  print("\n0 = Quit"
    + "\n1 = Look up a term"
    + "\n2 = Add a term"
    + "\n3 = Redefine a term"
    + "\n4 = Delete a term"
    + "\n5 = Display Dictionary"
  )

def is_integer(value):
  try:
   temp = int(value)
   return True
  except ValueError:
   return False

def validate(choice):
  if is_integer(choice) and 0 <= int(choice) <= 5:
    return int(choice)
  else:
    print("Input must be an integer between 0 and 5, inclusive")
    return validate(input("\nEnter Selection: "))

def lookup_term(dictionary):
  term = input("which term would you like to look up? ")
  if term in dictionary:
    print("{} : {}".format(term, dictionary.get(term)))
  else:
    print("Term does not exist, input 2 to add new term")

def redefine_term(dictionary):
  term = input("which term would you like to redefine? ")
  if term in dictionary:
    dictionary[term] = input("and its definition? ")
  else:
    print("Term does not exist, input 2 to add new term")

def add_term(dictionary):
  term = input("What term would you like to add? ")
  if term in dictionary:
    print("Already exists. To redfine input 3")
  else:
    dictionary[term] = input("and its definition? ")

def delete_term(dictionary):
  del dictionary[input('Which term would you like to delete? ')]

def process_request(choice, dictionary):
  if choice == 0:
    print("Thank you for using Stack Exchange Site Abbreviation!")
    quit()
  elif choice == 1:
    lookup_term(dictionary)
  elif choice == 2:
    add_term(dictionary)
  elif choice == 3:
    redefine_term(dictionary)
  elif choice == 4:
    delete_term(dictionary)
  else:
    print_dict(dictionary)

def main():
  site_dictionary = {
    'SO'  : 'Stack Overflow',
    'CR'  : 'Code Review',
    'LH'  : 'Lifehacks',
    '??'  : 'Puzzling',
    'SR'  : 'Software Recommendations',
    'SU'  : 'Super User',
    'M'   : 'Music: Practice & Theory',
    'RE'  : 'Reverse Engineering',
    'RPi' : 'Raspberry Pi',
    'Ro'  : 'Robotics'
  }

  print_dict(site_dictionary)
  print("\nWelcome to Stack Exchange Site Abbreviation Translator!")
  display_menu()
  while(True):
    process_request(validate((input("\nEnter Selection: "))), site_dictionary)

if __name__ == "__main__":
  main()
\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

Don't return validate from within validate, that's unnecessary recursion and you might end up accidentally hitting the maximum recursion level (as much as that shouldn't happen, what if the user just holds down enter?). Instead wrap the function in a while True loop. Since you have a return statement, you already have a mechanism to break the loop.

def validate(choice):
    while True:
        if is_integer(choice) and 0 <= int(choice) <= 5:
            return int(choice)
        else:
            print("Input must be an integer between 0 and 5, inclusive")
            choice= input("\nEnter Selection: ")

(Though I agree about adding the is_integer test into here)

Python implicitly concatenates neighbouring string literals, so in your menu print you actually don't need to use plus signs:

  print("\n0 = Quit"
        "\n1 = Look up a term"
        "\n2 = Add a term"
        "\n3 = Redefine a term"
        "\n4 = Delete a term"
        "\n5 = Display Dictionary"
  )

In redefine_term you don't do any validation on the new text being entered. Sure, the user can enter what they want but what if it's empty space? Do you want that. Maybe you do, but if not you could easily validate with an or:

dictionary[term] = input("and its definition? ") or dictionary[term]

If the input is an empty string it evaluates as False, meaning that Python then uses the other value in the A or B expression, defaulting back to the old value. If you wanted to prevent whitespace in general (eg tabs or spaces) then just add .strip() to the input call, to remove whitespace from the start and end of the result.

You have a bug in delete_term. If the user enters a non existent key it will raise a KeyError. You should probably handle it with a try except

try:
    del dictionary[input('Which term would you like to delete? ')]
except KeyError:
    print("That key does not exist.")
\$\endgroup\$
2
\$\begingroup\$

Some small improvements:

  1. print_dict method: use iteritems()

    for key, value in dictionary.iteritems():
    print("{} : {}".format(key, value))
    
  2. You can merge validate and is_integer. There are no benefits here gained by separating them in two different functions.

  3. For lookup_term, use either:

    value = dictionary.get(term)
    if value:
        print("{} : {}".format(term, value))
    else:
        print("Term does not exist, input 2 to add new term")
    

    Or you could use:

    if term in dictionary:
        print("{} : {}".format(term, dictionary[term]))
    else:
        print("Term does not exist, input 2 to add new term")
    

    The point I wanted to illustrate was use of dict.get()

  4. In delete_term, you are not validating for input when key is not in dictionary.

Some other things:

Since all functions work with same dictionary, they could have been grouped together under same class with dictionary being shared data member.

Also, your dictionary is case sensitive. Looking at your use-case, this is not something you want.

If at anytime you need to change menu, you would need to change validate function. Maybe there should be easier way to do this? (Instead of else denoting 5 in menu, else could work for something else?)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.