Number-to-word converter

My code works, but the aesthetic of the code seems to need some real work. The logic is simple enough:

• Create three lists of words that will be used throughout the conversion

• Create three functions:

• One for sub-1000 conversion

• Another for 1000-and-up conversion

• Another function for splitting and storing into a list of numbers above 1000.

First the code:

```
# Create the lists of word-equivalents from 1-19, then one for the tens group.
# Finally, a list of the (for lack of a better word) "zero-groups".

ByOne = [
"zero",
"one",
"two",
"three",
"four",
"five",
"six",
"seven",
"eight",
"nine",
"ten",
"eleven",
"twelve",
"thirteen",
"fourteen",
"fifteen",
"sixteen",
"seventeen",
"eighteen",
"nineteen"
]

ByTen = [
"zero",
"ten",
"twenty",
"thirty",
"forty",
"fifty",
"sixty",
"seventy",
"eighty",
"ninety"
]

zGroup = [
"",
"thousand",
"million",
"billion",
"trillion",
"quadrillion",
"quintillion",
"sextillion",
"septillion",
"octillion",
"nonillion",
"decillion",
"undecillion",
"duodecillion",
"tredecillion",
"quattuordecillion",
"sexdecillion",
"septendecillion",
"octodecillion",
"novemdecillion",
"vigintillion"
]

strNum = raw_input("Please enter an integer:\n>> ")

# A recursive function to get the word equivalent for numbers under 1000.

def subThousand(inputNum):
num = int(inputNum)
if 0 <= num <= 19:
return ByOne[num]
elif 20 <= num <= 99:
if inputNum[-1] == "0":
return ByTen[int(inputNum[0])]
else:
return ByTen[int(inputNum[0])] + "-" + ByOne[int(inputNum[1])]
elif 100 <= num <= 999:
rem = num % 100
dig = num / 100
if rem == 0:
return ByOne[dig] + " hundred"
else:
return ByOne[dig] + " hundred and " + subThousand(str(rem))

# A looping function to get the word equivalent for numbers above 1000
# by splitting a number by the thousands, storing them in a list, and
# calling subThousand on each of them, while appending the correc

Try to extract the different pieces of logic in functions to make things clearer. In your case, you have defined little functions but the place where you are using them is a bit in the middle of nowhere.

Here's what you could do :

def get_number_as_words(strNum):
  intNum = int(strNum)
  if intNum < 0:
      print "Minus", # I didn't see this at the beginning but I'll fix it at the end
      intNum *= -1
      strNum = strNum[1:]

  if intNum < 1000:
      return subThousand(strNum)
  else:
      return thousandUp(strNum)

Also, all these functions should be properly documented using docstrings.

If your code can potentially be used as a module, it is advised to put the part which actually does stuff in a function and guard the call to this function behind a if __name__ == "__main__": condition.
In your case, here's what I have :

def main():
  strNum = '95505896639631893' #raw_input("Please enter an integer:\n>> ")
  expected='ninety-five quadrillion, five hundred and five trillion, eight hundred and ninety-six billion, six hundred and thirty-nine million, six hundred and thirty-one thousand, eight hundred and ninety-three '
  print(get_number_as_words(strNum))
  print(expected)
  assert(get_number_as_words(strNum)==expected)

if __name__ == "__main__":
  main()

As I am messing around with your code, I use assert to detect quickly if I break the test case you have provided. Also, it shows something that we might want to fix on the long run : the trailing whitespace.

subThousand: You can use divmod to divide integers and get the quotient and the remainder.

subThousand: In successive if, else if, else if, etc, you do not need to check the same condition multiple times. Also, you can use assert to check that your function is used on proper values (described in the documentation).

subThousand: This function (and it might be the case for the others as well but I have to progress one function at a time) should probably be fed an integer and not a string. Operation on numbers should be enough for pretty much everything.

subThousand: You can use implicit evaluation of integers as boolean : 0 is False, anything else is True.

subThousand: As explained in Janne Karila's comment, it might be worth returning an empty string when the input is 0.

After taking into account the comments (except the last), the function looks like :

def subThousand(n):
    assert(isinstance(n,(int, long)))
    assert(0 <= n <= 999)
    if n <= 19:
        return ByOne[n]
    elif n <= 99:
        q, r = divmod(n, 10)
        return ByTen[q] + ("-" + subThousand(r) if r else "")
    else:
        q, r = divmod(n, 100)
        return ByOne[q] + " hundred" + (" and " + subThousand(r) if r else "")

I am a big fan of the ternary operator but it's purely personal. Also I used a recursive call to make things more consistent.

splitByThousands: By taking into account previous comments (types, divmode, etc), you can already get something more concise :

def splitByThousands(n):
    assert(isinstance(n,(int, long)))
    assert(0 <= n)    
    res = []
    while n:
        n, r = divmod(n, 1000)
        res.append(r)
    return res

thousandUp: The same comments are still relevant.

thousandUp: We maintain one list resArr and a number lenArr through the function but we can see that the property lenArr + len(resArr) + 1 == len(arrZero) is (almost) always true. One can easily get convinced of this or add assert(lenArr + len(resArr) + 1 == len(arrZero)) everywhere. Thus lenArr = len(arrZero) - len(resArr) - 1.

thousandUp: The block

wrd = subThousand(z) + " "
    zap = zGroup[len(arrZero) - len(resArr) - 1] + ", "
    if wrd == " ":
        break
    elif wrd == "zero ":
        wrd, zap = "", ""
    resArr.append(wrd + zap)

can be simplified a lot :

• there is not point in adding a whitespace to wrd at this stage.

• you do not need to use zGroup in all cases.

• as subThousand(X) is "zero" only if X==0, wrd == "zero" becomes z == 0

• as subThousand(X) is never an empty string, this test has no reason to be there. (If you ever perform the change described above, subThousand(X) will be empty when X==0 which is a case already handled).

Thus the block becomes :

if z:
        wrd = subThousand(z)
        zap = zGroup[len(arrZero) - len(resArr) - 1] + ", "
    else:
        wrd, zap = "", ""
    resArr.append(wrd + " " + zap)

which can then be rewritten :

if z:
        wrd_zap = subThousand(z) + " " + zGroup[len(arrZero) - len(resArr) - 1] + ", "
    else:
        wrd_zap = " "
    resArr.append(wrd_zap)

Then, because there is not point in adding spaces because all elements of the array already end with a space, we can do :

if z:
        wrd_zap = subThousand(z) + " " + zGroup[len(arrZero) - len(resArr) - 1] + ", "
    else:
        wrd_zap = ""
    resArr.append(wrd_zap)

thousandUp: Once the code is simplified, we realise tha

StackExchange Code Review Q#43744, answer score: 6