-1
\$\begingroup\$

I'm a Python newbie and I made this GUI dictionary using PyDictionary module, it took me long though, I can sense that the if conditions are really unpractical and ugly, but I couldn't find a way to shorten that part of the code, any recommendations will be highly appreciated.

from tkinter import *
from PyDictionary import PyDictionary

root = Tk()
root.geometry("450x550")
root.columnconfigure(0, weight=1)
root.config(bg="black")


def space():
    space = Label(text="", bg="black")
    space.grid()


def find_meaning():
    word = entry.get()
    dictionary = PyDictionary(word)
    definition = dictionary.getMeanings()
    print(definition)
    label = ""
    try:

        if "Verb" in definition[word]:
            label = "Verb: " + definition[word]["Verb"][0]
        if "Noun" in definition[word]:
            label = "Noun: " + definition[word]["Noun"][0]
        if "Adverb" in definition[word]:
            label = "Adverb: " + definition[word]["Adverb"][0]
        if "Noun" in definition[word] and "Adverb" in definition[word]:
            label = "Noun: " + definition[word]["Noun"][0] + " \n\n " + "Adverb: " + definition[word]["Adverb"][0]
        if "Noun" in definition[word] and "Adjective" in definition[word] and "Adverb" in definition[word]:
            label = "Noun: " + definition[word]["Noun"][0] + " \n\n " + "Adjective: " + definition[word]["Adjective"][0] + " \n\n " + "Adverb: " + definition[word]["Adverb"][0]
        if "Adjective" in definition[word]:
            label = "Adjective: " + definition[word]["Adjective"][0]
        if "Noun" in definition[word] and "Adjective" in definition[word] and "Adverb" in definition[word] and "Verb" in definition[word]:
            label = "Noun: " + definition[word]["Noun"][0] + " \n\n " + "Adjective: " + definition[word]["Adjective"][
                0] + " \n\n " + "Adverb: " + definition[word]["Adverb"][0] + " \n\n " + "Verb: " + definition[word]["Verb"][0]

        if "Verb" in definition[word] and "Noun" in definition[word]:
            label = "Noun: " + definition[word]["Noun"][0] + " \n\n " + "Verb: " + definition[word]["Verb"][0]
        if "Noun" in definition[word] and "Adjective" in definition[word]:
            label = "Noun: " + definition[word]["Noun"][0] + " \n\n " + "Adjective: " + definition[word]["Adjective"][0]
        if "Adjective" in definition[word] and "Verb" in definition[word]:
            label = "Adjective: " + definition[word]["Adjective"][0] + " \n\n " + "Verb: " + definition[word]["Verb"][0]

    except Exception:
        label = "Wrong word entered!"

    return label


def write():
    label.config(text=find_meaning())
    entry.delete(0, END)


space()
dic_text = Label(root, text="Dictionary", fg="#3dcc8e", bg="black", font=("arial", 15, "bold"))
dic_text.grid()

space()
entry = Entry(root, font=("times", 15, "bold"))
entry.grid()

space()
btn = Button(root, text="Find Meaning", command=lambda: [find_meaning(), write()])
btn.grid()

space()
label = Label(root, text="Translation", background="#3e3e3e",
              width=40, height=21, relief=FLAT, state=DISABLED, disabledforeground="#3dcc8e", wraplength=200,
              justify=LEFT)
label.grid()

root.mainloop()
\$\endgroup\$
2
  • \$\begingroup\$ The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$
    – BCdotWEB
    Nov 16 at 7:37
  • \$\begingroup\$ @Katkoota "hire some people to do that for you". The site is run by volunteers. However, I'm sure people would be happy to go the extra mile you're demanding if you pay them. \$\endgroup\$
    – Peilonrayz
    Nov 17 at 9:57
1
\$\begingroup\$

One way to clean up the Verb/Noun/Adverb/Adjective if blocks would be to handle each type independently of one another. Using a list to "collect" each definition grouped by type, you can then use .join to place \n\n in between each definition type.

def find_meaning():
    word = entry.get()
    dictionary = PyDictionary(word)
    definition = dictionary.getMeanings()
    # Set up an empty list to hold our definitions
    label_list = []
    if "Noun" in definition[word]:
        label_list.append("Noun: " + definition[word]["Noun"][0])
    if "Verb" in definition[word]:
        label_list.append("Verb: " + definition[word]["Verb"][0])
    if "Adjective" in definition[word]:
        label_list.append("Adjective: " + definition[word]["Adjective"][0])
    if "Adverb" in definition[word]:
        label_list.append("Adverb: " + definition[word]["Adverb"][0])
    # After each word form definition has been added,
    # join them together with two new lines in between each definition
    label = "\n\n".join(label_list)
    return label

Edit

I wanted to expand on the comment made by @FMc as it is still possible to shorten the code:

Also notice that definition[word] is used many times. A short convenience variable would lighten up the visual weight of the code, enhancing readability: defin = dictionary.getMeanings()[word]

Modifying my original code to include these suggestions could look like the following:

def find_meaning():
    word = entry.get()
    dictionary = PyDictionary(word)
    defin = dictionary.getMeanings()[word]
    # Set up an empty list to hold our definitions
    label_list = []
    if "Noun" in defin:
        label_list.append("Noun: " + defin["Noun"][0])
    if "Verb" in defin:
        label_list.append("Verb: " + defin["Verb"][0])
    if "Adjective" in defin:
        label_list.append("Adjective: " + defin["Adjective"][0])
    if "Adverb" in defin:
        label_list.append("Adverb: " + defin["Adverb"][0])
    # After each word form definition has been added,
    # join them together with two new lines in between each definition
    label = "\n\n".join(label_list)
    return label

Notice how the readability is not lost, but possibly improved so long as the intent is easy to understand to other reviewers.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Wow, this is great and easy to understand, thank you very much! \$\endgroup\$
    – Katkoota
    Nov 15 at 13:07
  • 1
    \$\begingroup\$ The suggestions here are good (+1). Also notice that definition[word] is used many times. A short convenience variable would lighten up the visual weight of the code, enhancing readability: defin = dictionary.getMeanings()[word]. \$\endgroup\$
    – FMc
    Nov 15 at 18:28
1
\$\begingroup\$

Sure; you've got the right idea. Start by writing the code as repetitively as possible, then factor out the repetitive bits. Your first three lines are of the form

if partOfSpeech in definition[word]:
    label = "{}: ".format(partOfSpeech) + definition[word][partOfSpeech][0]

So, factor that out into a function!

def maybeSetSingleLabel(partOfSpeech):
    if partOfSpeech in definition[word]:
        label = "{}: ".format(partOfSpeech) + definition[word][partOfSpeech][0]

maybeSetSingleLabel("Verb")
maybeSetSingleLabel("Noun")
maybeSetSingleLabel("Adverb")

And then refactor into a loop:

for partOfSpeech in ["Verb", "Noun", "Adverb"]:
    maybeSetSingleLabel(partOfSpeech)

And then repeat the process for the next few lines:

def maybeSetDoubleLabel(pos1, pos2):
    if pos1 in definition[word] and pos2 in definition[word]:
        label = "\n\n".join(
            "{}: ".format(pos1) + definition[word][pos1][0],
            "{}: ".format(pos2) + definition[word][pos2][0],
        )

maybeSetDoubleLabel("Noun", "Adverb")
maybeSetSingleLabel("Adjective")
maybeSetDoubleLabel("Verb", "Noun")
maybeSetDoubleLabel("Noun", "Adjective")
maybeSetDoubleLabel("Adjective", "Verb")

...Hey, that single "Adjective" case is out of order! We should move it up with the other three single-label cases.

And then we could do the same for triple and quadruple labels. But maybe at this point we should look at reducing the repetition again. We have some repetition here parameterized on the number of labels as well as their string values. So let's make a general-purpose maybeSetLabels function!

def maybeSetLabels(poses):
    if all(pos in definition[word] for pos in poses):
        label = "\n\n".join([
            "{}: ".format(pos) + definition[word][pos][0] for pos in poses
        ])

maybeSetLabels(["Verb"])
maybeSetLabels(["Noun"])
maybeSetLabels(["Adverb"])
maybeSetLabels(["Adjective"])
maybeSetLabels(["Noun", "Adverb"])
maybeSetLabels(["Verb", "Noun"])
maybeSetLabels(["Noun", "Adjective"])
maybeSetLabels(["Adjective", "Verb"])
maybeSetLabels(["Noun", "Adjective", "Adverb"])
maybeSetLabels(["Noun", "Adjective", "Adverb", "Verb"])

...Hmm, you're missing some combinations. For example, ["Noun", "Verb", "Adverb"] is missing. Let's just have the computer generate the combinations for us! Computers are good at that.

def all_subsets_of(words):
    for r in range(len(words)):
        for subset in itertools.combinations(words, r+1):
            yield subset

for poses in all_subsets_of(["Adjective", "Adverb", "Noun", "Verb"]):
    maybeSetLabels(poses)

And now that there's only one call to maybeSetLabels, we can re-inline its code:

def all_subsets_of(words):
    for r in range(len(words)):
        for subset in itertools.combinations(words, r+1):
            yield subset

label = ""
for poses in all_subsets_of(["Adjective", "Adverb", "Noun", "Verb"]):
    if all(pos in definition[word] for pos in poses):
        label = "\n\n".join([
            "{}: ".format(pos) + definition[word][pos][0] for pos in poses
        ])

There are other ways to tackle the problem, too, depending on the shape of your data. For example, if every key in definition[word] is a part of speech that you're trying to put into the label, then you could just do this one line:

label = "\n\n".join([
    "{}: ".format(pos) + definition[word][pos][0] for pos in definition[word]
])
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you very much for your great and detailed answer, I'm sure it will benefit a lot of advanced developers!, however, it is a bit hard for me to understand all that :( but thanks anyway sir <3 \$\endgroup\$
    – Katkoota
    Nov 15 at 13:06

Your Answer

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

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