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

I've been trying to solve this problem with python and I think I've figured out a way that covers all the cases that are possible given the mathematical definition of an odd number. In order to be sure, I would like to check if my code is correct given your own criteria.

counter = 0
odd = []

while counter < 10:
    x = int(input("Enter a number: "))
    if abs(x)%2 != 0:
        odd.append(x)
    counter += 1

if len(odd) == 0:
    print("No odd number was entered")
else:
    print("The largest odd number is:", max(odd))
\$\endgroup\$
4
  • 2
    \$\begingroup\$ Doesn't even run, has multiple indentation errors. \$\endgroup\$ Aug 19 '20 at 2:01
  • 13
    \$\begingroup\$ Why is storing all numbers required ? Keep only a max_odd and update it if a bigger odd number appears. \$\endgroup\$
    – aki
    Aug 19 '20 at 8:15
  • 1
    \$\begingroup\$ @anki I was thinking the EXACT same thing \$\endgroup\$
    – clockw0rk
    Aug 19 '20 at 9:25
  • 1
    \$\begingroup\$ it does run, at least it does in my IDLE. Thanks @anki, I didn't know how to tell python to store only the maximum odd while running. \$\endgroup\$ Aug 19 '20 at 10:46
19
\$\begingroup\$

For your current program we can improve a couple things:

  1. Rename odd to odds (since it is a list).
  2. Use not odds instead of len(odds) == 0 (see How do I check if a list is empty? for a reason as to why this is preferred).
  3. Delete counter. Since we only use counter in the while condition, we can actually replace the whole while with for _ in range(10).
  4. Follow PEP 8. For example, using 4 spaces for each indentation level.

Factoring in all these changes, we get:

odds = []

for _ in range(10):
    x = int(input("Enter a number: "))
    if abs(x) % 2 != 0:
        odds.append(x)

if not odds:
    print("No odd number was entered")
else:
    print("The largest odd number is:", max(odds))

But we can also improve the efficiency of this program. Right now we keep track of all odd numbers, before choosing the max. This means that the space complexity is O(N). We can change this to O(1) by keeping track of the largest odd number like so:

max_odd = None

for _ in range(10):
    x = int(input("Enter a number: "))

    if abs(x) % 2 != 0:
        max_odd = x if max_odd is None else max(max_odd, x)

if max_odd is None:
    print("No odd number was entered")
else:
    print("The largest odd number is: ", max_odd)

Note that we use None to signify that no odd number has been entered so far, in which case upon an odd number being entered we set max_odd to x directly. Otherwise, we set max_odd to max(max_odd, x).

For this type of program you will not notice the increase in efficiency due to reducing the space complexity. But learning to recognize where these reductions are possible will allow you to see the same patterns in programs where it does matter.

There is finally one more thing you can do. If you want to allow the program to keep accumulating numbers in the event that a str is accidentally typed which cannot be parsed as a number (such as ""), we can use a try / except wrapped in a while like so:

while True:
    try:
        x = int(input("Enter a number: "))
        break
    except ValueError:
        continue

This would replace:

x = int(input("Enter a number: "))

in the original code. This would keep prompting the user to type a str that is parsable as an int until they do. Since this is all happening in the same iteration of the for, the count of numbers they get to type (10 in our case) would not be reduced.

\$\endgroup\$
8
  • 4
    \$\begingroup\$ The second-to-last code block could be written without the x_is_valid variable. The are two approaches: (1) change loop condition to while True: and x_is_valid = True to break; (2) use the counter variable in the original solution and replace x_is_valid = True with counter += 1. The second approach also avoids the need of the inner loop entirely. \$\endgroup\$
    – GZ0
    Aug 19 '20 at 1:29
  • \$\begingroup\$ @GZO Great catch, edited. \$\endgroup\$ Aug 19 '20 at 1:39
  • 3
    \$\begingroup\$ @NeisySofíaVadori A common pattern you'll find in programming is that you have a collection of things (whether it's a list, a range of numbers, etc); and that you want to loop over each item in it in order. This is so common that for loops are standard to let you do this faster, rather than having to write all the boilerplate logic to define an iteration variable, ensure it is within the range, and then increment / move that iteration variable at the end of the loop: instead, you're able to write all of this in a single line. \$\endgroup\$ Aug 19 '20 at 11:52
  • 2
    \$\begingroup\$ for loops are used more over an iterator (list, dict, set, etc) or when you have some constant amount of repeat times. while loops are used when you know only a stop/continue condition. @NeisySofíaVadori \$\endgroup\$ Aug 19 '20 at 11:54
  • 1
    \$\begingroup\$ @Ben In Python 2 that would have worked, but in Python 3 max(None, x) (the very first iteration) raises TypeError. See stackoverflow.com/questions/8961005/…. \$\endgroup\$ Aug 19 '20 at 17:04
6
\$\begingroup\$

Adding to the previous review:

  • When x is an integer, abs(x) % 2 is equivalent to x % 2 in Python. The output of the modulo operator % has the same sign as the second operand.
  • When running code outside a method / class, it is a good practice to put the code inside a main guard. See here for more explanation.

In Python 3.8, the code can be shortened using the assignment operator := together with max function.

if __name__ == "__main__":
    # Number generator
    num_gen = (o for _ in range(10) if (o := int(input("Enter a number: "))) % 2 != 0)
    max_odd = max(num_gen, default=None)
    if max_odd is None:
        print("No odd number was entered")
    else:
        print(f"The largest odd number is: {max_odd}")

Wrapping int(input("Enter a number: ")) into a function provides better readability:

def read_input() -> int:
    return int(input("Enter a number: "))

if __name__ == "__main__":
    num_gen = (o for _ in range(10) if (o := read_input()) % 2 != 0)
    max_odd = max(num_gen, default=None)
    if max_odd is None:
        print("No odd number was entered")
    else:
        print(f"The largest odd number is: {max_odd}")

Another variant that handles invalid user inputs is as follows:

def read_input() -> int:
    while True:
        try:
            return int(input("Enter a number: "))
        except ValueError:
            continue

if __name__ == "__main__":
    try:
        max_odd = max(o for _ in range(10) if (o := read_input()) % 2 != 0)
        print(f"The largest odd number is: {max_odd}")
    except ValueError:
        # Since read_input() no longer raises ValueError, the except
        # statement here only handles the cases where max() gets no inputs
        print("No odd number was entered")
\$\endgroup\$
4
  • 8
    \$\begingroup\$ Shorter isn't necessarily better. I certainly need more time to read this code to understand what it does, compared to Mario's or the OP's code. \$\endgroup\$
    – Florian F
    Aug 19 '20 at 7:24
  • 2
    \$\begingroup\$ I'd use max(..., default=None) and an if max_odd is not None: else: statement, instead of the try-except, since a ValueError could come from a non-integer input and doesn't truly mean no odd value was entered. \$\endgroup\$
    – AJNeufeld
    Aug 19 '20 at 17:54
  • \$\begingroup\$ @AJNeufeld Thanks. I've updated my post. \$\endgroup\$
    – GZ0
    Aug 20 '20 at 13:21
  • \$\begingroup\$ @FlorianF The generator expression is a bit too long to comprehend. I've edited my post. \$\endgroup\$
    – GZ0
    Aug 20 '20 at 13:30
3
\$\begingroup\$

May I ask what programming language did you practice before python?
I want to mention a one-liner for this:

max(l,key=lambda x:(x%2,x))

assuming you already have l somehow inputed, like

s='Enter a number: '
l=[int(input(s)) for i in range(10)]

How does the code work? It looks for maximum of key(x) for x in l and returns such x. Key here is the lambda function that returns tuple (1,x) for odd x and (0,x) for even x. Tuples are compared left to right, e.g. (1,x)>(0,y) for every x and y. So we are just saying "give me the maximum of l, assuming that an odd number is always larger than an even number".

So all program will look like

s='Enter a number: '
l=[int(input(s)) for i in range(10)]
m=max(l,key=lambda x:(x%2,x))
if m%2:
    print('The largest odd number is: %d'%m)
else: #the greatest is even, therefore no odd numbers
    print('No odd number was entered')

Short, nice and easy, as python.

But I agree that a try-except block around int(input()) form the accepted answer is useful, along with no pre-storing the entire list of odd values.

I only wanted to demonstrate the paradigm of functional programming in python, when you just tell python 'I want that done (e.g. a maximum value)' and it does it for you, you don't need to explain how should it do it.

Thanks for reading.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks @AlexeyBurdin, I found your code really elegant and I definetely appreciate that. Regarding your question, I started learning python just some weeks ago and is the first programing language I'm learning, but I'm studing physics so my math knowledge definitely helps. \$\endgroup\$ Aug 19 '20 at 10:52
3
\$\begingroup\$

I'll try to build on the last suggestion of the accepted answer.

while True:
    try:
        x = int(input("Enter a number: "))
        break
    except ValueError:
        continue

I definitely endorse this suggestion, it allows your program to handle invalid input gracefully instead of just crashing.

However, it creates an usability problem. The user who just typoed a letter into a number probably did not notice it. They'll think they got the intended number in, proceed with the next, and then get confused at the end, when they think they got all numbers in, but the computer is still asking for the next one.

Better to give them feedback:

while True:
    try:
        x = int(input("Enter a number: "))
        break
    except ValueError:
        print("Invalid number will be ignored.")
        continue

... or even better, print their typoed number back at them:

while True:
    try:
        inputString = input("Enter a number: ")
        x = int(inputString)
        break
    except ValueError:
        print("Invalid number will be ignored: {}".format(inputString))
        continue

I would also consider keeping the full list of valid numbers entered, not just odd ones, and printing them all back at the user before the result, to give them a last chance to spot typos. They can, after all, have mistyped a valid but unintended number. Note that this would increase memory usage, and some would consider it over-communication.

print("Numbers provided are: {}".format(all_valid_numbers_inputted))
if not odds:
    print("No odd number was entered")
else:
    print("The largest odd number is:", max(odds))

If you do this, the next step would be to get rid of the "odds" variable, and figure out the largest odd directly from the full list.

\$\endgroup\$
1
\$\begingroup\$

The key point here: each step in the process does just one simple thing. You build programs that way -- one incremental, tightly defined step at a time. Don't mix everything in a jumble -- e.g. a loop where we interact with a user while also making conversions and calculations needed later.

def as_int(s):
    try:
        return int(s)
    except Exception:
        return 0

N = 3
MSG = 'Enter number: '

replies = [input(MSG) for _ in range(N)]  # Interact.
nums = [as_int(r) for r in replies]       # Convert.
odds = [n for n in nums if n % 2]         # Compute.

if odds:                                  # Report.
    print(max(odds))
\$\endgroup\$
4
  • \$\begingroup\$ Silently replacing a typo with a zero is no good. And you can't even fix it by adding a loop to as_int, since at that point it's already too late, all input has been done. That's a real downside of how you split the steps. \$\endgroup\$ Aug 25 '20 at 8:40
  • \$\begingroup\$ @superbrain Sure, in some contexts; in others, not so much. If the input-stage needs typo fixing/validation, it can be added easily, during that stage. But those are details the OP can sort out. My general point stands. \$\endgroup\$
    – FMc
    Aug 25 '20 at 14:54
  • \$\begingroup\$ as_int is an absolute no-go. input validation has to be in the interact stage. any silent and hidden assumption what-the-user-may-want is a no-go. that's what exceptions are for - unless fixed in the interact stage. if you cannot control the interact stage you shall implement an explicit validation stage. \$\endgroup\$
    – stefan
    Aug 26 '20 at 7:44
  • \$\begingroup\$ @stefan You are mistaken. First, neither of us know the purpose of the OP's larger program -- input validation needs might be irrelevant or unusual. Second, input validation can easily happen during the input stage: wrap the input() call in a function and do whatever is necessary. But that validation need not necessarily involve conversion of string to integer. It could involve such conversion, but my answer's main point is aimed elsewhere -- namely, the correct and important lesson of organizing your programs into discrete, readily understood steps. \$\endgroup\$
    – FMc
    Aug 26 '20 at 14:36

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.