0
\$\begingroup\$

My code does what it's suppose to do but the code is failing Pylint checks. After pasting my code into Pylint checker, it says that Too many branches (15/12) and its failed me on the Pylint checks.

How can this be cleaned up?

"""Docstring"""
import words

def nicer_englishify_word(word):
    """Docstring"""
    wordx = word.lower()
    wordy = ""
    if wordx.endswith('moo'):
        vowel = True
    else:
        vowel = False
    if vowel == True:
        wordy1 = wordx[-3] + wordx[:-3]
        wordy2 = wordx[:-3]
        if wordy1 in words.get_word_list():
            wordy11 = True
        else:
            wordy11 = False
        if wordy2 in words.get_word_list():
            wordy22 = True
        else:
            wordy22 = False
        if wordy11 == wordy22:
            if wordy11 == True:
                wordy = "sametrue"
            else:
                wordy = "samefalse"
        else:
            if wordy22 == False:
                wordy = "[" + wordx[-3] + wordx[:-3] + "]"    
            else:
                wordy = "[" + wordx[:-3] + "]"
        if wordy == "sametrue":
            wordy = wordx[-3] + wordx[:-3] + " or " + wordx[:-3]
            wordy = "<" + wordy + ">"
        if wordy == "samefalse":
            wordy = wordx[-3] + wordx[:-3] + " or " + wordx[:-3]
            wordy = "(" + wordy + ")"
    else:
        wordy = wordx[-3] + wordx[:-3]
    return wordy


def nicer_englishify_sentence(sentence):
    """docstring"""
    stringlist = sentence.split()
    result = "" 
    for item in stringlist:
        result = result + nicer_englishify_word(item) + " " 
    result = result[:-1]
    return result
\$\endgroup\$
3
  • 3
    \$\begingroup\$ Well, pylint doesn't like that you're using so many ifs ans elses. That doesn't mean your code is wrong. pylint doesn't check for correctness. \$\endgroup\$
    – L3viathan
    Apr 30, 2015 at 13:29
  • 1
    \$\begingroup\$ Why don't you use elif statements ? \$\endgroup\$
    – tema
    Apr 30, 2015 at 13:29
  • 1
    \$\begingroup\$ Welcome to Code Review. Please add some description of what the intended purpose of your code is. (See How to Ask.) \$\endgroup\$ Apr 30, 2015 at 18:38

3 Answers 3

6
\$\begingroup\$

If true, true. If false, false.

Your code is filled with situations like this:

if someCondition:
    variable = True
else:
    variable = False

You can replace all those occurrences with a simple assignment:

variable = someCondition

If, and if not

Additionally, there's no need to add == True in a comparison. The Python way of x == False is to do not x

Conditional ternary operator

if wordy11:
    wordy = "sametrue"
else:
    wordy = "samefalse"

Can be written as:

wordy = "sametrue" if wordy11 else "samefalse"

Variable names

What on earth is wordy? What type is it? It seems to be one variable used for all kinds of things. I'd recommend making these separate variables and using more descriptive variable names.

Result

This will clean up your code a bit at least, to this point:

"""Docstring"""
import words


def nicer_englishify_word(word):
    """Docstring"""
    wordx = word.lower()
    wordy = ""

    if wordx.endswith('moo'):
        wordy1 = wordx[-3] + wordx[:-3]
        wordy2 = wordx[:-3]
        wordy11 = wordy1 in words.get_word_list()
        wordy22 = wordy2 in words.get_word_list()
        if wordy11 == wordy22:
            wordy = "sametrue" if wordy11 else "samefalse"
        else:
            if not wordy22:
                wordy = "[" + wordx[-3] + wordx[:-3] + "]"    
            else:
                wordy = "[" + wordx[:-3] + "]"
        wordy = wordx[-3] + wordx[:-3] + " or " + wordx[:-3]
        wordy = "<" + wordy + ">" if wordy == "sametrue" else "(" + wordy + ")"
    else:
        wordy = wordx[-3] + wordx[:-3]
    return wordy
\$\endgroup\$
5
  • \$\begingroup\$ @Kevin Those are mostly probably better as ternary expressions anyway, like wordy = "sametrue" if wordy11 else "samefalse", or better yet wordy = "same" + str(wordy11).lower() \$\endgroup\$ Apr 30, 2015 at 13:50
  • \$\begingroup\$ @TheBlackCat Done. I'm not using Python very often but I think what I made it should be correct. (there is one more place that technically can be changed to a ternary, I know) \$\endgroup\$ Apr 30, 2015 at 13:59
  • \$\begingroup\$ You might need to use variable = bool(someCondition) if someCondition wasn't boolean to begin with. \$\endgroup\$ Apr 30, 2015 at 15:37
  • \$\begingroup\$ You took out the explaining variable vowel, which was the closest thing that the code had to some kind of documentation! \$\endgroup\$ Apr 30, 2015 at 18:40
  • \$\begingroup\$ @200_success Okay, true. Although that could easily be added again. I don't see the connection between vowel and wordx.endswith('moo') though, so perhaps it's more confusion than documentation? \$\endgroup\$ Apr 30, 2015 at 19:17
2
\$\begingroup\$

In addition to Simon's changes, here are some more improvements:

  1. You recalculate wordy1 and wordy2 a lot. You should define them once and re-use them whenever you need.
  2. wordy1 uses wordy2. So you can define wordy1 in terms of wordy2.
  3. You can use something like '[{}]'.format(foo) instead of something like '[' + foo + ']' to make your code clearer (in my opinion).
  4. If you use format, you don't have to do so immediately. So you can define your words, and define your separators, then combine them later.
  5. You can short-circuit your last else clause by putting it near the beginning and having it return the result. This allows you to unindent a large block of code.
  6. You never use the initial wordy you create. Get rid of it
  7. You re-use variable names for different things. That is confusing. You should use more descriptive names (I have taken a shot at that in my example below).
  8. If get_word_list is expensive, you should probably only do it once and store it as a global variable. You can then pass it as a default argument to speed up access. This also makes the code more flexible since you can specify an alternative word list if you want.
  9. You should use boolean operations rather than == to check if multiple booleans are the same. In this case, you can reverse your if test and use xor, which will will return True if and only if exactly one is True. Python doesn't explicitly have xor. Since you are sure in this case that both are bool, you can use ^ instead.
  10. 'sametrue' and 'samefalse' are just used as test values later. You can remove these by moving the test into the code block where you originally create them.

So here is a somewhat simplified version:

"""Docstring"""
import words

WORD_LIST = words.get_word_list()


def nicer_englishify_word(word, word_list=WORD_LIST):
    """Docstring"""
    word = word.lower()

    word2 = word[:-3]
    word1 = word[-3] + word2

    if not word.endswith('moo'):
        return word1

    word1isin = word1 in word_list
    word2isin = word2 in word_list
    if word1isin ^ word2isin:
        inner = word2 if word2isin else word1
        outer = "[{}]"
    else:
        inner = "{} or {}".format(word2, word1)
        outer = "<{}>" if word1isin else "({})"
    return outer.format(inner)
\$\endgroup\$
1
\$\begingroup\$

3 the perfect number

The number 3 is repeated 11 times in your code. What does that 3 stand for? Your code is kind of mysterious: I can't decide, but I am sure you can. Define a constant:

MEANINGFUL_NAME = 3
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Everywhere it is used is as part of a slice, and there are only two such slices that are re-done over and over. It would be easier to just do each slice once, put each in a variable, and then use those variables. \$\endgroup\$ May 5, 2015 at 9:27

Your Answer

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