2
\$\begingroup\$

Ask user to input 10 integers and then print the largest odd number that was entered. If no odd number was entered, print a message to that effect.

The above exercise is from chapter 2 of Computation and Programming Using Python by John Guttag. So far, the book has covered variable assignment, print function, conditional branching, and loops. The book is about problem solving and uses Python only as a tool to solve problems and it is meant to be used with Python 2.x. I need to know if my solution to the exercise can be "straightened up" in any way.

numbers_entered = []

for _ in range(10):
    number = raw_input('Enter an integer: ')
    if number.isdigit():
        number = int(number)
        numbers_entered.append(number)
    else:
        print 'That was not an integer!'

odd_numbers = []

for item in numbers_entered:
    if item % 2 != 0:
        odd_numbers.append(item)

if odd_numbers:
    print 'The largest odd number entered was %s.' % str(max(
            odd_numbers))
else:
    print 'No odd number was entered.'
\$\endgroup\$
  • \$\begingroup\$ I assume this is based on feedback from this question? That’s fine (and posting follow-up code as a separate question is encouraged), but if so, it would be good to link back to that question and highlight the differences. \$\endgroup\$ – alexwlchan Sep 27 '15 at 19:13
7
\$\begingroup\$

Bug

At the end of the first loop, how many numbers will be in numbers_entered? 10? Hopefully. In reality: 10 minus invalid entries, and I'm not sure that was your intention.

This way the invalid entries will be skipped, and there will always be 10 entries in list:

while True:
    number = raw_input('Enter an integer: ')
    if number.isdigit():
        number = int(number)
        numbers_entered.append(number)
        break
    else:
        print 'That was not an integer!'

Handling invalid input

The user input should be valid, invalid inputs should be the exception. So you can simplify the handling of invalid user inputs using exceptions:

try:
    number = int(raw_input('Enter an integer: '))
    numbers_entered.append(number)
except ValueError:
    print 'That was not an integer!'

See also the glossary, and search for the word "forgiveness" in it.

Using list comprehensions

List comprehensions are awesome. The loops could be replaced by list comprehensions (with the help functions), for example:

def read_int():
    while True:
        try:
            return int(raw_input('Enter an integer: '))
        except ValueError:
            print 'That was not an integer!'

numbers = [read_int() for _ in range(10)]

odd_numbers = [item for item in numbers if item % 2]

if odd_numbers:
    print 'The largest odd number entered was {}.'.format(max(odd_numbers))
else:
    print 'No odd number was entered.'

Prefer .format(...) for formatting

Instead of formatting with %, the new preferred way is using .format(...):

print 'The largest odd number entered was {}.'.format(max(odd_numbers))

Notice that there's no need to call str(...) when using this style.

Readability

This statement would have been better on a single line:

print 'The largest odd number entered was %s.' % str(max(
        odd_numbers))
\$\endgroup\$
  • \$\begingroup\$ I think your first code sample is missing some code on the break line -- it should test to see if there are 10 members of the list. \$\endgroup\$ – Snowbody Sep 28 '15 at 1:45
  • \$\begingroup\$ @janos, that was awesome! My follow-up code is at codereview.stackexchange.com/questions/105887/… \$\endgroup\$ – srig Sep 28 '15 at 6:09
  • 1
    \$\begingroup\$ @Snowbody my first sample is to go inside his for loop \$\endgroup\$ – Stop ongoing harm to Monica Sep 28 '15 at 6:14
  • \$\begingroup\$ @janos, too many calls to read_int()? I'm wondering if that's all right. \$\endgroup\$ – srig Sep 28 '15 at 7:51
  • \$\begingroup\$ Yes, nothing wrong with that \$\endgroup\$ – Stop ongoing harm to Monica Sep 28 '15 at 8:30
0
\$\begingroup\$

as for the checking for 10 input numbers,

why not try

while len(numbers_entered) != 10:
    number = raw_input('Enter an integer: ')
    if number.isdigit():
        number = int(number)
        numbers_entered.append(number)
        break
    else:
        print 'That was not an integer!'
\$\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.