Print the largest odd number

Question

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.

I've written this code as my solution to the above exercise from chapter 2 of Computation and Programming Using Python by John Guttag. The book is meant to be used with Python 2.x. I need to know if the code can be "straightened up" in any way. So far, the book has covered variable assignment, print function, conditional branching, and loops.

largestOddNumber = None
for _ in range(10): # '_' is a throw-away var
 number = int(raw_input('Enter an integer: '))
 if number % 2 != 0 and number > largestOddNumber:
 largestOddNumber = number
print largestOddNumber or 'No odd number was entered.'

• Snowbody: I don't think it's worth it for me to make an answer for this, but when you're writing a real program (not just an exercise) it's best to separate the input-output calls and the logic of the code. This code separates easily into two pieces:(1) Get 10 integers, and (2) calc the maximum. The code gets the 10 integers, passes them into a calculation function and then prints the return value with desired formatting. The calc function should have no Input/Output (that's all in the caller). So you separate the I/O from the logic.
• kangalio: Assuming your input is a list (like Snowbody suggested), you can do max(number for number in input_list if number % 2 == 1). Almost as readable as plain English, compact, easy to maintain and reason about, and probably pretty fast

Answer 1

Code looks good! I only have minor/pedantic comments:

for _ in range(10): # '_' is a throw-away var

That comment is useless, the naming of the variable as _ is sufficient to indicate that we don't intend on using it. Also, in python2.7, range() returns a full list of all the values, which can get pretty inefficient as you go up, so you should prefer to use its lazier cousin, xrange():

for _ in xrange(10):

Then this:

print largestOddNumber or 'No odd number was entered.'

While I do like that or in python, rather than returning a bool returns the first argument that isn't False/None, it's a little difficult to read. Now, the code is perfectly correct for odd numbers but would actually be wrong for even numbers - if 0 ended up being the largest even number, we'd print the string. So I find it simpler to just be explicit:

if largestOddNumber is not None:
 print largestOddNumber
else:
 print 'No odd number was entered'

But really, the code is fine.

Answer 2

Your code is fine.

I have a few nitpick-style comments, that’s all:

• In this if condition:

  if number % 2 != 0 and number > largestOddNumber:
  

  I would turn the test for being odd into an affirmative one: number % 2 == 1, rather than a negation of the test for being even. I would also add parentheses around the two conditions – they’re not required by Python, but I think they often make code easier to read:

  if (number % 2 == 1) and (number > largestOddNumber):
  

• The Python style guide is PEP 8, which includes conventions for variable names.

  Variables should be named using lowercase_with_underscores (also called snake_case), with the exception of classes, which use CamelCase. So your variable should really be largest_odd_number.

• If I don’t enter any odd numbers, I get an English sentence:

  No odd number was entered.
  

  whereas if I enter some numbers, I just get a number back:

  37
  

  It would be nice if I got an English sentence to describe the good output:

  The largest odd number entered was 37.
  

Finally, while I know you’re explicitly writing Python 2.x code, here’s a bit of advice for making this code future-proof for Python 3:

• In your if statement, you do a comparison between the user-entered number and None (until you set the variable largest_odd_number).

  Python 2 will let you mix types that way, and treats None as being less than all numbers, including float(-inf). In Python 3, the same comparison would throw a TypeError:

  Python 3.5.0 (v3.5.0:374f501f4567, Sep 12 2015, 11:00:19)
  >>> None < 5
  Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  TypeError: unorderable types: NoneType() < int()
  

  You can expand the if statement to be more future-proof if you do a comparison to None first. This is also a little more explicit about how the program works, I think:

  if (number % 2 == 0) and \
   (largest_odd_number is None or number > largest_odd_number)
  

  That check would work on both Python 2.x and 3.x.

Answer 3

You don't perform any input validation, so if I enter letters, or nothing at all, then you'll get errors. You could avoid this by wrapping the input in a try except and then looping over that until valid input is entered, like this:

for _ in range(10): # '_' is a throw-away var
 while True:
 try:
 number = int(raw_input('Enter an integer: '))
 break
 except ValueError:
 print ("Please enter integers only.")
 if number % 2 != 0 and number > largestOddNumber:
 largestOddNumber = number

This way, if there's an error calling int, the user will be reminded to only enter integers. But if int runs correctly then it will reach the break statement and continue running with the valid input.

• srig: thank you. Please look at my follow-up code at codereview.stackexchange.com/questions/105887/…

Answer 4

I can't comment here yet, but incorporating @snowbody's and @alexwlchan's answers, can you do this to future proof it?

import sys
if sys.version_info.major == 2:
 input = raw_input
prompt = "Enter ten integers, separated by a space."
found_max = "The largest odd number entered was {}."
no_odds = "No odd number was entered."
ints = [int(i) for i in input(prompt).split()]
if len(ints) != 10:
 raise ValueError("Your input was not good.")
odds = [i for i in ints if i % 2]
print(found_max.format(max(odds)) if odds else no_odds)

• Snowbody: it's okay..it still mixes IO and calculation in the same line (doesn't have a separate function for the calculation). Also, you can control the input better with a loop, something like prompt = "Enter integer {} of ten." ints = [input(prompt.format(x)) for x in xrange(10)]
• srig: @jeannassar, if i equals zero and because 0%2 equals zero, this even number gets into the list of odds
• Jean Nassar: @srig, I just tried it. It does not. It only adds the number if i % 2 is not equal to zero.
• Jean Nassar: @Snowbody, what do you mean calculation here? The splitting and type conversion? I guess odds = [int(i) for i in ints if int(i) % 2] in this case.
• srig: @JeanNassar, I've just found out it only adds odds. Thank you