5
\$\begingroup\$

This is part of a project I made a couple of years ago and was looking at again. Its purpose is to check text for words that an early reader (about a late kindergarten or first grade level) would likely be unable to read. It does this by checking for CVC (consonant-vowel-consonant) and CVCE (consonant-vowel-consonant-E) words as well as slight variations on the CVC that begin or end with common two-letter combinations. (For example, an early reader should have little trouble with a word like "clap."). It also checks for words on the pre-primer Dolch lists. (These are common "sight words" that kids should immediately recognize.)

Is there any way to make this more efficient? In particular, as I look at it, there are an awful lot of "if" statements. Is there some way to cut down on those, or does it even matter?

Also, do you think this is readable as is, or is there anything I should do to format it better?

Here is the "core" of the code:

    # Check if a word could be CVC or CVCE 

def check_pattern(text):
    while len(text) >= 3:
        l= [text[0], text[1], text[2]]
        if l[0] in vowels:
            return False
        elif l[1] not in vowels:
            return False
        elif l[2] in vowels:
            return False
        else:
            return True

# Check if a word is CVC.
def check_CVC(text):
    pattern = check_pattern(text)
    if pattern == False:
        return False
    elif len(text) != 3:
        return False
    else: 
        return True

# Check if a word is CVCE.        
def check_CVCE(text):
    pattern = check_pattern(text)
    if pattern == False:
        return False
    elif len(text) != 4:
        return False
    elif text[3] != "e":
        return False
    else:
        return True

#Check if a word begins with a consonant blend or digraph. If so, read as if one letter
def check_bb (text):
    if len(text) >= 3:
        st= text[0]+text[1]
        if st in IC:
            p = list(text)
            del p[1]
            text = "".join(p)
            return text
        else:
            return text
    else:
            return text

#Check if a word ends with a consonant blend, digraph, or double letter. If so, read as if one letter
def check_eb (text):
    if len(text) >= 3:
        end = text[-2]+text[-1]
        if end  in FC:
            p = list(text)
            del p[-1]
            text = "".join(p)
            return text
        else:
            return text
    else:
            return text
# Strip off any punctuation at the end of a word.        
def no_punct(text):
    punct = ["!", ".", "?", ",", ";", ":"]
    if text[-1] in punct:
        p = list(text)
        del p[-1]
        text = "".join(p)
        return text
    else:
        return text
#Check if a word is preprimer, primer, or easily decodable
def check_readable(text):
    word = no_punct(text)
    lc = word.lower()
    merge_start = check_bb(lc)
    merge_end = check_eb(merge_start)
    CVC = check_CVC(merge_end)
    CVCE = check_CVCE(merge_end)


    if CVC == True:
        return True
    elif CVCE == True:
        return True
    elif lc.lower() in preprimer:
        return True
    elif lc.lower() in primer:
        return True
    else:
        return False
#Flag words in a text that are not easily decodable, preprimer, or primer
def flag(text):
    words = text.split()
    i = 0
    while i < len(words):
        r = check_readable(words[i])
        if r == False:
            words[i] = "*" + words[i] + "*"
            i += 1
        else:
            i += 1
    text = " ".join(words)
    return text

For reference, here is the link to the entire code, including the letter/blend/word lists and a Tkinter GUI: https://github.com/casinclair/Reader

\$\endgroup\$
4
\$\begingroup\$

The code you presented is not readable and definitely needs refactoring.

You can simplify several code constructions and apply the "Extract Variable" refactoring method to improve readability of the program:

You have a complicated expression. Put the result of the expression, or parts of the expression, in a temporary variable with a name that explains the purpose.

For instance, check_pattern() may use the variable unpacking and condition merging, resulting into an explicit and understandable code. I've also renamed it to is_cvc:

def is_cvc(text):
    """Checks if a word is CVC: consonant-vowel-consonant."""
    has_3_characters = len(text) == 3
    if not has_3_characters:
        return False 

    first, second, third = text[:3]
    first_is_consonant = first not in vowels
    second_is_vowel = second in vowels
    third_is_consonant = third not in vowels

    return first_is_consonant and second_is_vowel and third_is_consonant

Then, we can reuse this for the cvce check as well:

def is_cvce(text):
    """Checks if a word is CVCE: consonant-vowel-consonant-E."""
    has_4_characters = len(text) == 4
    if not has_4_characters:
        return False

    ends_with_e = text[-1].lower() == 'e'
    is_also_cvc = is_cvc(text[:3])

    return ends_with_e and is_also_cvc

Also note that I've moved the function comments into proper docstrings.

Note that I've also applied .lower() to the last character to handle both e and E.


In particular, as I look at it, there are an awful lot of "if" statements. Is there some way to cut down on those, or does it even matter?

Yes, it matters a lot.

The code is much more often read than written. The more if statements and branches you have, the more complicated, less readable and more difficult to maintain it is. There are some code complexity measurement metrics like "Cyclomatic code complexity" or "Maintainability Index" that may give a rough idea of how readable and maintainable the code is. It is also important to note that a more complex code tend to have more bugs in it:

Some studies find a positive correlation between cyclomatic complexity and defects: functions and methods that have the highest complexity tend to also contain the most defects.


Also, since you are dealing with natural language here and the input comes in a form of a text, see if switching to nltk for word tokenization would be easier and more robust (especially, in terms of handling punctuation).

\$\endgroup\$
  • \$\begingroup\$ Thank you. I appreciate the links--that will give me a good bit of reading to do. \$\endgroup\$ – Celeste Sinclair Jun 18 '17 at 3:44

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.