Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm trying to become more proficient in Python and have decided to run through the Project Euler problems.

In any case, the problem that I'm on (17) wants me to count all the letters in the English words for each natural number up to 1,000. In other words, one + two would have 6 letters, and so on continuing that for all numbers to 1,000.

I wrote this code, which produces the correct answer (21,124). However, I'm wondering if there's a more pythonic way to do this.

FYI on the code: I have a function (num2eng) which translates the given integer into English words but does not include the word "and".

for i in range (1, COUNTTO + 1):
    container = num2eng(i)
    container = container.replace(' ', '')
    print container
    if i > 100:
        if not i % 100 == 0:
            length = length + container.__len__()  + 3 # account for 'and' in numbers over 100
        else:
            length = length + container.__len__() 
    else:
        length = length + container.__len__() 
    print length

So there is some repetition in my code and some nesting, both of which seem un-pythonic. Any suggestions on ways to simplify it are very much appreciated!

Thank you.

Edit: I'm specifically looking for ways to make the script shorter and with simpler loops, I think that's my biggest weakness in general. The answers below are perfect, thanks everyone!

Edit #2: Changed the title.

share|improve this question
    
Your question isn't very specific; it's pretty open ended. We like specific programming questions here with a clear answer. –  Hiroto Feb 21 '13 at 22:36
5  
Don’t call magic methods directly, just use len(container). –  poke Feb 21 '13 at 22:39
    
I know it doesn't relate to the programming question, but integers should never be written out or spoken with "and." "And" is used only for fractions. 1234 is "one thousand twenty-four" with no "and", but 23.5 is twenty-three and one half. –  askewchan Feb 21 '13 at 23:07
    
@askewchan: that's far from a universal standard, and I definitely don't think it rises to the level of "should never be". Even Americans, I believe, tend to include the "and" when writing on cheques. Unfortunately continued discussion would be off-topic, but for my part, let a thousand (and one) flowers bloom. –  DSM Feb 22 '13 at 0:56
    
@Hiroto Edited to make more specific, thanks! –  Josh Saint Jacque Feb 22 '13 at 5:23
show 2 more comments

migrated from stackoverflow.com Feb 22 '13 at 0:45

This question came from our site for professional and enthusiast programmers.

3 Answers

up vote 2 down vote accepted

Here are a few things:

  • container.__len__() – You should never call magic methods directly. Just use len(container).
  • As you don’t know to what amount you need to increment (i.e. your COUNTTO), it would make more sense to have a while loop that keeps iterating until you reach your final length result:

    while length < 1000:
         # do stuff with i
         length += # update length
         i += 1
    
  • You could also make use of itertools.count:

    length = 0
    for i in itertools.count(1):
        length += # update length
        if length > 1000:
            break
    
  • not i % 100 == 0 – When you already have a operator (==) then don’t use not to invert the whole condition, but just use the inverted operator: i % 100 != 0
  • Also having additional counting logic outside of your num2eng does not seem to be appropriate. What does that function do exactly? Shouldn’t it rather produce a real number using a complete logic? Ideally, it should be as simple as this:

    length = 0
    for i in itertools.count(1):
        length += len(num2eng(i))
        if length > 1000:
            break
    
share|improve this answer
    
I think the reason to have some logic outside of num2eng is to avoid counting the letters in 'hundred' and 'thousand' so many times. –  askewchan Feb 21 '13 at 23:10
    
@askewchan Huh? That doesn’t make much sense to me, as you still do that. Also I’d expect a function num2eng to return a complete written English representation of the number I pass in. –  poke Feb 21 '13 at 23:13
    
You're correct. The function num2eng returns the English representation of the numbers provided. For instance, 102 becomes one hundred two. It's doesn't include "and" though and the scope of the challenge required "and" to be included (e.g., one hundred and two), hence the addition. But it would makes sense to modify num2eng instead of making up for it here. –  Josh Saint Jacque Feb 22 '13 at 5:19
    
Thank you so much for this feedback, this is exactly what I was looking for! –  Josh Saint Jacque Feb 22 '13 at 5:25
add comment

The more pythonic version:

for i in range(1, COUNTTO + 1):
    container = num2eng(i).replace(' ', '')
    length += len(container)
    if i % 100:
        length += 3   # account for 'and' in numbers over 100

print length

code golf:

print sum(len(num2eng(i).replace(' ', '')) + bool(i > 100 and i % 100) * 3
          for i in range(1, COUNTTO + 1))

:)

Don't do that in real code, of course.

share|improve this answer
    
Thank you, this is really great feedback! –  Josh Saint Jacque Feb 22 '13 at 5:24
add comment

You can make it a bit shorter

length += len(container) + 3 if (i > 100 and i % 100) else 0
share|improve this answer
    
i % 100 can't be 0 for 1 <= i <= 100. –  Pavel Anossov Feb 21 '13 at 22:46
    
but for 1 <= i <= 100 there's no 'and' in the word –  Duc Feb 21 '13 at 22:47
    
Ah yes. Excuse me. –  Pavel Anossov Feb 21 '13 at 22:49
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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