4
\$\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.

This is a follow-up code on feedback from Find the largest odd number

This time I've taken care of a bug related to the number of integers entered and tried to handle invalid input if any. Please let me know if the code still can be tidied up in any way. I'm a beginner programmer.

print "You'll be asked to enter 10 integers."
print 'The program will look for odd numbers, if any.'
print 'Then, it will print the largest odd number.'
print 'Press Enter to start.'
raw_input('-> ')

numbers_entered = []

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

    if len(numbers_entered) == 10:
        break

odd_numbers = [item for item in numbers_entered if item % 2 == 1]

if odd_numbers: 
    print 'The largest odd number entered was {}.'.format(max(odd_numbers))
else:
    print 'No odd number was entered.'
\$\endgroup\$
  • 2
    \$\begingroup\$ You can move the break condition into the while statement: while len(numbers_entered) <= 10). \$\endgroup\$ – alexwlchan Sep 28 '15 at 6:14
  • \$\begingroup\$ You could check if number is odd and if it's the greatest one so far in the while statement. Just an alternative to your way of doing it, slightly more efficient memory-wise. \$\endgroup\$ – mostruash Sep 28 '15 at 7:33
  • \$\begingroup\$ @mostruash, please look at codereview.stackexchange.com/questions/105821/… That's my previous post on the same problem. \$\endgroup\$ – srig Sep 28 '15 at 7:55
4
\$\begingroup\$

Be careful of changing back and forth from ' and ". I can see that you probably used the double quotes just so you could include an apostrophe, but I think it's best to stay consistent and use them throughout since you never need to print any "'s later in the program.

print "You'll be asked to enter 10 integers."
print "The program will look for odd numbers, if any."

Also, you don't need to keep the list of odd_numbers if all you want is the highest one. So you could just directly get the max value:

max_value = max([item for item in numbers_entered if item % 2 == 1])

And this way, you don't even need to build a list. You can instead pass something called a generator expression directly to max. It's like a list comprehension, but it doesn't create a full list. It can be used in both for loops and functions that lake lists, like max does.

However, this would raise a ValueError if you had no odd numbers in the list, as max can't take an empty sequence. But you could just use this as your test instead of if odd_numbers.

try: 
    max_value = max(item for item in numbers_entered if item % 2 == 1)
    print "The largest odd number entered was {}.".format(max_value)
except ValueError:
    print "No odd number was entered."
\$\endgroup\$
3
\$\begingroup\$

Firstly, seeing as you want to loop until the length of "numbers_entered" is 10, I would change:

while True:

to

while len(numbers_entered) < 10:

so that you don't have to include that if statement.

Also, in the same way that you make sure that only integers are added, you can also make sure that only odd numbers are added. Add the following code into your try statement between "number = int(raw_input('Enter an integer: '))" and "numbers_entered.append(number)"

if number % 2 == 0:
    raise

You might then want to change your exception from

except ValueError:
    print 'That was not an integer.'  

to just

except:
    print 'That was not an odd integer'

This way the exception will be raised if the inputted value is either not an integer or it's odd. This would be your finished code.

print "You'll be asked to enter 10 integers."
print 'The program will look for odd numbers, if any.'
print 'Then, it will print the largest odd number.'
print 'Press Enter to start.'
raw_input('-> ')

numbers_entered = []

while len(numbers_entered) < 10:
    try:
        number = int(raw_input('Enter an integer: '))
        if number % 2 == 0:
            raise
        numbers_entered.append(number)      
    except:
        print 'That was not an odd integer.'

odd_numbers = [item for item in numbers_entered if item % 2 == 1]
print 'The largest odd number entered was {}.'.format(max(odd_numbers))
\$\endgroup\$
  • 2
    \$\begingroup\$ Welcome to Code Review! Good note on the while loop. Though I think that the OP wants to consider an even number as valid input and later filter it out. ie. The brief says 10 integers, not specifically 10 odd integers. \$\endgroup\$ – SuperBiasedMan Sep 28 '15 at 9:31
  • 3
    \$\begingroup\$ You really shouldn't be using bare except statements unless you have a good reason to not specify the exception types. \$\endgroup\$ – Kevin Brown Sep 28 '15 at 11:55
  • \$\begingroup\$ I disagree, I think you should use bare except statements unless you have a good reason to specify the exception types, there is no such reason in this application. \$\endgroup\$ – Robbie Coyne Sep 28 '15 at 14:45
  • 2
    \$\begingroup\$ The reason is, what if an unexpected exception occurs? Your code will happily catch it and behave wrongly. If an exception occurs that your code's not able to handle, it should let the exception on by in hopes that the caller is able to handle it. \$\endgroup\$ – Snowbody Sep 28 '15 at 16:02
  • \$\begingroup\$ In what way could there be an incorrect exception in this case? The only way it could give an exception is if the inputted value is either odd or not an integer \$\endgroup\$ – Robbie Coyne Sep 28 '15 at 17:34
1
\$\begingroup\$

You can do this without storing the inputs and looping through them afterwards. You only need to remember whether or not an odd number was entered and which one is the largest. My Python is weak so I'll use pseudocode:

var hasUserEnteredOddNumber = false
var largestOddNumber = 0

for(var i = 0; i < 10; i++)
{
    var input = ReadIntFromConsole()
    if(input mod 2 == 1)
    {
        if(hasUserEnteredOddNumber)
            largestOddNumber = max(largestOddNumber, input)
        else
            largestOddNumber = input
        hasUserEnteredOddNumber = true
    }
}

if(hasUserEnteredOddNumber)
    Print("The largest odd number was " & largestOddNumber)
else
    Print("No odd number was entered")

You'll need a ReadIntFromConsole function. It'll be something like this:

function int ReadIntFromConsole()
{
    while(true)
    {
        string input = ReadFromConsole()
        if(input is an integer)
            return ConvertToInt(input)
        print("That was not an integer")
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ your code fails if the largest odd integer is negative \$\endgroup\$ – Snowbody Sep 28 '15 at 16:00
  • \$\begingroup\$ @Snowbody, I can't believe I made that mistake. I'll fix it. \$\endgroup\$ – user2023861 Sep 28 '15 at 16:04
  • \$\begingroup\$ Fixed the problem. \$\endgroup\$ – user2023861 Sep 28 '15 at 16:06
  • 1
    \$\begingroup\$ looks good, would you mind clicking the uparrow for my comment? \$\endgroup\$ – Snowbody Sep 28 '15 at 16:10

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.