5
\$\begingroup\$

I'm very new to programming and am admittedly embarrassed sharing my code for critique. This code works and produces the correct answer to the Project Euler Problem 17, which asks how many letters there are when the numbers from one to one thousand are written out in words (excluding spaces and punctuation).

I was hoping I could get some critique on how to make it less ugly and more streamlined, or even approach the problem from a totally new angle so I can better learn from this solution.

# the final list that holds the string lengths
addList = [] 

# dictionary holding integer:corresponding word pairs
numbersDict = { 
0:"zero",
1:"one",
2:"two",
3:"three",
4:"four",
5:"five",
6:"six",
7:"seven",
8:"eight",
9:"nine",
10:"ten",
11:"eleven",
12:"twelve",
13:"thirteen",
14:"fourteen",
15:"fifteen",
16:"sixteen",
17:"seventeen",
18:"eighteen",
19:"nineteen",
20:"twenty",
30:"thirty",
40:"forty",
50:"fifty",
60:"sixty",
70:"seventy",
80:"eighty",
90:"ninety"
}

### There has to be an easier way to do all this below ###

def numberLetters(num):

    letters = ""

    if 0 < num <= 20:
        letters += numbersDict[num]

    if 21 <= num <= 99:
        a,b = divmod(num, 10)
        if b == 0:
            letters += numbersDict[a*10]
        else:
            letters += numbersDict[a*10] + numbersDict[b]

    if 100 <= num <= 999:
        if num % 100 == 0:
            letters += numbersDict[int(num / 100)] + "hundred"
        else:
            digit = int(num / 100)
            num = num - digit * 100
            if 0 < num <= 20:
                letters += numbersDict[digit] + "hundredand" + numbersDict[num]
            if 21 <= num <= 99:
                a,b = divmod(num, 10)
                if b == 0:
                    letters += numbersDict[digit] + "hundredand" + numbersDict[a*10]
                else:
                    letters += numbersDict[digit] + "hundredand" + numbersDict[a*10] + numbersDict[b]
    if num == 1000:
        letters += "onethousand"

    return letters

for i in range(1,1001):
    addList.append(len(numberLetters(i)))
print(sum(addList))
\$\endgroup\$
  • 1
    \$\begingroup\$ Did you look on the project Euler answer forums? There's probably a bunch of really clean Python implementations there. Since your answer works, you should be able to view them. \$\endgroup\$ – michaelsnowden Oct 17 '15 at 6:09
  • \$\begingroup\$ thanks @michaelsnowden, I didn't even think of that. Will do going forward \$\endgroup\$ – Jai Kamat Oct 17 '15 at 14:19
1
\$\begingroup\$

Aside from the repetitive logic in the code to handle numbers greater than a hundred, the solution is not bad. There are many observations that could lead to improvements, though.

  • You don't need to build the strings themselves; you just need to sum their lengths. In particular, since strings are immutable, += is relatively costly. (Your solution is still fast enough, though.)
  • The challenge never requires you to treat 0 as "zero". You can therefore simplify some logic by mapping 0 to an empty string.
  • The ranges 0 < num <= 20, 21 <= num <= 99, 100 <= num <= 999, and num == 1000 are mutually exclusive.
  • You can reduce repetition by using recursion.

Solution

numbers_dict = {
    n: len(word) for (n, word) in {
        0: "",
        1: "one",
        2: "two",
        3: "three",
        4: "four",
        5: "five",
        6: "six",
        7: "seven",
        8: "eight",
        9: "nine",
        10: "ten",
        11: "eleven",
        # ...
        90: "ninety",
    }.iteritems()
}

def num_length(n):
    if n <= 20:
        return numbers_dict[n]
    elif n < 100:
        ones = divmod(n, 10)
        return numbers_dict[10 * tens] + num_length(rest)
    elif n < 1000:
        hundreds, rest = divmod(n, 100)
        return num_length(hundreds) + len("hundred") + (
            len("and") + num_length(rest) if rest else 0
        )
    else:
        thousands, rest = divmod(n, 1000)
        return num_length(thousands) + len("thousand") + num_length(rest)

print(sum(num_length(n) for n in range(1, 1001)))

You could also use numbers_dict for memoization:

def num_length(n):
    if n in numbers_dict:
        return numbers_dict[n]
    elif n < 100:
        tens, rest = divmod(n, 10)
        numbers_dict[n] = numbers_dict[10 * tens] + num_length(rest)
        return numbers_dict[n]
    elif n < 1000:
        hundreds, rest = divmod(n, 100)
        return num_length(hundreds) + len("hundred") + (
            len("and") + num_length(rest) if rest else 0
        )
    else:
        thousands, rest = divmod(n, 1000)
        return num_length(thousands) + len("thousand") + num_length(rest)
\$\endgroup\$

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.