1
\$\begingroup\$

I wrote simple code to spell the number below 1,000,000,000. It works perfectly as per my intention. However, I do think my code is a little bit messy since I only able to imagine of using decision statements in my function/method.

I'm still learning in writing Python function/method code. I will be glad if anyone here can review my code and comment on how to improve it.

def main():
    value = int(input("\nPlease enter a positive integer < 1000000000 : "))
    if value < 100:
        print(processDigit(value))
    elif value > 100:
        print(intName(value))

# turns a number into its English name
def intName(number):
    part = number
    nameMillion = ""
    nameThousand = ""
    nameHundred = ""

    if part >= 1000000:
        divide = part // 1000000
        part = part % 1000000
        divide1 = divide // 100
        part1 = divide % 100
        nameMillion = digitName(divide1) + " hundred" + processDigit(part1) + " million "

    if part >= 1000:
        divide = part // 1000
        part = part % 1000
        divide1 = divide // 100
        part1 = divide % 100
        nameThousand = digitName(divide1) + " hundred" + processDigit(part1) + " thousand "

    if part >= 100:
        divide = part // 100
        part = part % 100
        nameHundred = digitName(divide) + " hundred" + processDigit(part)

        return nameMillion + nameThousand + nameHundred


# process the three digit number
def processDigit(number):
    part = number
    name = ""

    if part >= 20:
        name = name + " " + tensName(part)
        part = part % 10

    elif part >= 10:
        name = name + " " + teenName(part)
        part = 0

    if part > 0:
        name = name + " " + digitName(part)

    return name

# turns a digit into its English name
def digitName(digit):
    if digit == 1: return "one"
    if digit == 2: return "two"
    if digit == 3: return "three"
    if digit == 4: return "four"
    if digit == 5: return "five"
    if digit == 6: return "six"
    if digit == 7: return "seven"
    if digit == 8: return "eight"
    if digit == 9: return "nine"
    return ""


# turns a number between 10 and 19 into its English name
def teenName(number):
    if number == 10: return "ten"
    if number == 11: return "eleven"
    if number == 12: return "twelve"
    if number == 13: return "thirteen"
    if number == 14: return "fourteen"
    if number == 15: return "fifteen"
    if number == 16: return "sixteen"
    if number == 17: return "seventeen"
    if number == 18: return "eighteen"
    if number == 19: return "nineteen"
    return ""


# gives the name of tens part of a number between 20 and 99
def tensName(number):
    if number >= 90: return "ninety"
    if number >= 80: return "eighty"
    if number >= 70: return "seventy"
    if number >= 60: return "sixty"
    if number >= 50: return "fifty"
    if number >= 40: return "fourty"
    if number >= 30: return "thirty"
    if number >= 20: return "twenty"
    return ""

# start the program
main()
\$\endgroup\$
  • \$\begingroup\$ So we currently have the year " hundred two thousand ", right? I don't think that code actually works as expected yet. As a matter of fact - your code doesn't even reach the return statement for 2016! \$\endgroup\$ – Ext3h Sep 19 '16 at 7:41
  • \$\begingroup\$ There's a useful module out there which allows you to convert an int to english: pip install num2words you should give it a try. It will reduce your code by at least 40% \$\endgroup\$ – Grajdeanu Alex. Sep 19 '16 at 7:53
  • \$\begingroup\$ @Ext3h ..I did test it for random number and realize some number did not work as your comment. I'm still learning and I do realize that I still need to improvise my code. However, as per my question is there any way or method to simplify the function. \$\endgroup\$ – niese Sep 19 '16 at 7:56
  • \$\begingroup\$ Unfortunately, the requirement is that the code work first. Once it works the way that you want, then we can help simplify it. \$\endgroup\$ – mdfst13 Sep 19 '16 at 13:23
12
\$\begingroup\$
def main():
    value = int(input("\nPlease enter a positive integer < 1000000000 : "))
    if value < 100:
        print(processDigit(value))
    elif value > 100:
        print(intName(value))

You extracted logic which should have been encapsulated inside your function into the main function. That should never happen. If intName can't handle values between 1 and 99 itself, it's not living up to its name.


def intName(number):
    part = number
    nameMillion = ""
    nameThousand = ""
    nameHundred = ""

    if part >= 1000000:
        divide = part // 1000000
        part = part % 1000000
        divide1 = divide // 100
        part1 = divide % 100
        nameMillion = digitName(divide1) + " hundred" + processDigit(part1) + " million "

    if part >= 1000:
        divide = part // 1000
        part = part % 1000
        divide1 = divide // 100
        part1 = divide % 100
        nameThousand = digitName(divide1) + " hundred" + processDigit(part1) + " thousand "

    if part >= 100:
        divide = part // 100
        part = part % 100
        nameHundred = digitName(divide) + " hundred" + processDigit(part)

        return nameMillion + nameThousand + nameHundred

This function is simply wrong, for multiple reasons. Let's step through them one by one:

The return statement is sometimes unreachable

Check your indentation. Currently the return statement is only reachable if number % 1000 >= 100.

Numbers >=1,000 and <100,000 do exist

Your forgot to handle the case when the number doesn't contain the term "hundred". The same issue exists with numbers >= 1,000,000 and <100,000,000.

Writing the same code 3x in a row, with the same mistakes

You are using different constants, but the logic is mostly identical. The copy & paste is pretty obvious. And it had exactly the effect you would expect: You copied the mistakes as well.

This would have been easily solved by extracting the common parts of the logic properly.

0 is a number

While you don't say "zero hundred" or "zero thousand", "zero" alone is a valid number and should be supported properly.


Structuring the code properly

nameMillion = digitName(divide1) + " hundred" + processDigit(part1) + " million "

Why did you bother writing this explicitly for every potence of 1,000?

Have another look at how the numbers in the english language are constructed, and try to find ALL patterns.

If you that, you realize that for every number, every potence of 1,000 is formed identically:

[([digit "hundred" ]decimal) "suffix"]

The only exception to that is the part below 1,000 which has no suffix. However, a missing suffix can simply be approximated by an empty suffix instead.

So the logic approach is to write yourself a helper function which correctly formats every number from 0 to 999, because that is the range you need commonly.

Your next realization should be that it is easier to start constructing a number from the LSB, than from the MSB, since you then only ever need one set of constant numbers to extract the relevant part of the number.

Once you have that function, intName() can be written much clearer:

def intName(number):
    output = "";
    suffixes = ["", "thousand", "million", "billion"]

    if number == 0
        return "zero"

    while number > 0:
        number, part = divmod(number, 1000);
        if part > 0
            output = intThousandName(part) + " " + suffixes[0] + " " + output
        suffixes.pop(0)

    return output.strip()

The strip() at the end removes the trailing white space, resulting from the empty suffix, respectively the concatenation of the first valid suffix with the empty string.

\$\endgroup\$
  • 2
    \$\begingroup\$ Good advices overall, although I would avoid poping items from the list. Instead, I'd rather write for suffix in suffixes: number, part = divmod(number, 1000); ... and be able to define suffixes as a constant somewhere. \$\endgroup\$ – 409_Conflict Sep 19 '16 at 12:26
  • \$\begingroup\$ Excellent. This what I'm looking for. Really appreciate your highlight on how to simplified the function. I vote your as an answer. \$\endgroup\$ – niese Sep 20 '16 at 1:43
4
\$\begingroup\$

Your functions for the Low Numbers (<20) should use a dictionary:

digits = {1: "one",
          2: "two",
          ...
          19: "nineteen"}
def digitName(digit): 
    try:
        return digits[digit]
    except KeyError:
        return ""
# or

def digitName(digit): 
    return digits.get(digit, "")

For the numbers below 100 you could use a list:

tens = [(90, "ninety"), (80, "eighty"), ..., (20, "twenty")])

def tensName(number): 
    if number >= 100:
        return ""
    for ten, name in tens:
        if number >= ten:
            return name

You could even combine these two functions:

def lowIntName(number): 
    if number >= 100:
        return ""
    if number < 20:
        return digits.get(number, "")
    for ten, name in tens:
        if number >= ten:
            return name + digits.get(number % 10, "")
    return ""

The final return will only be reached for negative numbers...

\$\endgroup\$
  • \$\begingroup\$ Great advice. I will look into the dictionary and list for this. \$\endgroup\$ – niese Sep 20 '16 at 1:27

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