6
\$\begingroup\$

I'm making my way through the Intro to Computation and Programming test by John Guttag and he gives these small exercises to practice gearing up into computational thinking. As of Ch.2, we've covered basic input and print commands as well as some simple iteration stuff.

The question is as follows:

Write a piece of code that asks the user to input 10 integers, and then prints the largest odd number that was entered. If no odd number was entered, it should print a message to that effect.

counter = 0
ans = 0
even=0
while counter < 10:
    x=int(raw_input('Please enter an integer: '))
    if x%2!=0:          #Checks if the input is odd 
        if ans < x:     #If it is, is the input larger than the held value for 'ans'?
            ans = x     #If yes, replace the value for 'ans' with the input
            counter +=1 #Move the counter up by one, ending at 10
    else:               #If it wasn't odd, add 1 to the even counter and to the integer counter, keeping ans the same
        even+=1
        counter+=1
if even == 10:          #if all numbers entered were even, this counter would be 10, and thus would give this response
    print 'All numbers were even'
else:                   #if at least 1 odd number was entered, the even counter would not be 10 and it would print the answer
    print 'the largest odd number entered was ', ans

Is there a more efficient way of writing this out? I wonder if I can word my if statements to include more parameters to cut down on the number of if/else statements? Also keep in mind this text uses python 2.7 and that I'm not super far into the book. I'm mainly trying to get the whole computational thinking thing down.

\$\endgroup\$
  • \$\begingroup\$ I have edited your tags. I'm not sure you really wanted to put machine-learning there because your question isn't related to it. If you feel like this edit wasn't good you can rollback it. Otherwise I think this is a great first question, congratulations and welcome to Code Review :) \$\endgroup\$ – IEatBagels May 29 at 16:31
  • \$\begingroup\$ Thank you! I appreciate you looking out. \$\endgroup\$ – TheBestDakota May 29 at 16:51
  • 2
    \$\begingroup\$ If you're just starting to learn Python now, I suggest learning Python 3. Python 2 is obsolete. \$\endgroup\$ – 200_success May 30 at 4:35
1
\$\begingroup\$

Python 2.x vs. Python 3.x

As a start, welcome to code review and since this is your first time here, well done, you did a good job here for a first post. One of the most confusing topics for beginner programmers is to choose which version of Python to learn. However there are more benefits to learn Python 3.x since it's the current version of Python and very soon there will be no future security or bug fixes for Python 2.x. and most of the developers nowadays are using Python 3.x, and as I'm familiar with the book you indicated, I know it's written in Python 2.x therefore I suggest you try to look things up and try to apply the same logic you encounter in exercises in Python 3.x (you will just be using a slightly different syntax).

Besides Python 3.x contains a bunch of new features that you'll eventually start using:

  • the print function
  • f strings
  • integer division
  • and many other features

Style

Here's a reference style guide (pep8) https://www.python.org/dev/peps/pep-0008/, the official Python guide to style you could read and here are a few comments about style:

  • If an assignment has a right hand side, then the equality sign should have exactly one space on both sides.

    even=0

    x=int(raw_input('Please enter an integer: '))

    if x%2!=0:

  • Use inline comments sparingly.

  • An inline comment is a comment on the same line as a statement. Inline comments should be separated by at least two spaces from the statement. They should start with a # and a single space.
  • Use f-strings for better formatting.

ex: print(f'largest odd: {odd_num}')

The if elif else statements:

An else statement can be combined with an if statement. An else statement contains the block of code that executes if the conditional expression in the if statement resolves to 0 or a FALSE value.

The else statement is an optional statement and there could be at most only one else statement following if.

The use of functions and lists:

You can use functions in programming to bundle a set of instructions that you want to use repeatedly or that, because of their complexity, are better self-contained in a sub-program and called when needed. That means that a function is a piece of code written to carry out a specified task. To carry out that specific task, the function might or might not need multiple inputs. When the task is carried out, the function can or can not return one or more values.

You can also use lists when you have more than a few items to do operations on. A list is a data structure in Python that is a mutable, or changeable, ordered sequence of elements. Each element or value that is inside of a list is called an item. Just as strings are defined as characters between quotes, lists are defined by having values between square brackets [ ]

The code can be simplified as follows:

def get_odd_max(n):
    """Return maximum odd number from n numbers, print a message for no odds"""
    odds = []
    evens = []
    for _ in range(n):
        # the use of try and except to catch illegal types without stopping the code execution.
        try:
            number = int(input('Enter an integer: '))
            if number % 2 == 0:
                evens.append(number)
            if number % 2 != 0:
                odds.append(number)
        except ValueError:
            print('Expected int.')
    if odds:
        # f-strings for a cleaner code
        # usually a function does not print anything(it's for the sake of the example)
        print(f'Maximum odd value: {max(odds)}')
        return max(odds)
    else:
        print('No odds found.')
        return 0


if __name__ == '__main__':
    print(get_odd_max(10))
\$\endgroup\$
1
\$\begingroup\$

Bug

If the user enters the values 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, ... they would expect the program to eventually stop asking me for input, but it won't. When the input is odd, the counter variable only increments when the value entered is larger than ans. The increment statement should be outdented one level, so it executes even when an odd number is not greater than ans:

    if x%2!=0:
        if ans < x:
            ans = x
        counter +=1    # <-- outdented to level of "x%2 != 0" test
    else:
        even+=1
        counter+=1

DRY: Don't Repeat Yourself

In contrast to WET (Write Everything Twice).

You have two counter += 1 statements, one in each path of the if ... else statement. Since you are counting up by one in both paths, this code should be moved out of the if ... else statement, so it can be written just once:

    if x%2!=0:
        if ans < x:
            ans = x
    else:
        even+=1

    counter += 1       # Executed regardless of which branch of if/else is taken.

In programming, there are 3 kinds of numbers: zero, one, and many. Zero and one are generally fine when written as constants through-out a program. "Many" is an exception; you don't want to write it more than once.

If you are asked to change the program to allow up to 20 integer to be input, you would have to change two statements: while counter < 10: and if even == 10:. This is frowned up on; when you have more than one, it is easy to miss one. One approach is to create a constant for each different kind of "many" value:

LIMIT = 10
# ...

while count < LIMIT:
    # ...

if even == LIMIT:
    # ...
else:
    # ...

Now, you just have to change the LIMIT = 10 statement if you want to change the number of values to be input.

But there is another way. Instead of checking if all the values were even (even == LIMIT), we could count the number of odd values:

counter = 0
ans = 0
odd = 0
while counter < 10:
    x = int(raw_input('Please enter an integer: '))
    if x % 2 != 0: 
        if ans < x:
            ans = x
            odd += 1
    counter += 1

if odd > 0:
    print 'the largest odd number entered was ', ans
else:
    print 'All numbers were even'

Now we are counting the number of odd values, and the test at the end checks if any odd numbers were given. And "any odd numbers" simply means the count of odd numbers is greater than zero ... which is an "ok" constant to write directly into the code.

Sentinels

We still may have a bug. If the user enters -3, 2, -5, 4, 6, -7, -8, 4, -10, -8, the program will reply:

the largest odd number entered was 0

  1. 0 isn't "odd".
  2. We never entered the value 0.

The problem here stems from initializing ans = 0, and hoping that some odd value will be larger than that value, so it will be overwritten.

A better approach is to initialize ans to a sentinel value, and when the first odd value is encountered, update ans to that value instead of checking whether the first odd value is larger than ans. Actually, 0 is a perfectly fine sentinel value, when looking for an odd value, but None is a more common value:

counter = 0
ans = None                           # Sentinel value
while counter < 10:
    x = int(raw_input('Please enter an integer: '))
    if x % 2 != 0: 
        if ans is None or ans < x:   # Check now includes sentinel value
            ans = x
    counter += 1

if ans is not None:
    print 'the largest odd number entered was ', ans
else:
    print 'All numbers were even'

Note that the sentinel value performs double duty. If ans is still the sentinel value at the end of the loop, then no odd values were found. The odd count is unnecessary and has been eliminated.

Alternatively, you could initialize ans to a value smaller than any value the user could possibly enter. Unfortunately, Python's integer range is not bounded, so they could enter any arbitrarily large negative integer. However, we're not limited to integers, so we could initialize ans = float("-inf"). Any integer the user enters will be greater than that, so the sentinel would not need to be checked inside the loop. The test at the end could use if not math.isinf(ans): to detect if any odd values were entered.

Loop Like a Native

The expression "Loop Like a Native" comes from a PyCon 2013 presentation, which contains advance topics, like generators. Caution: sharp learning curve ahead if you follow that link.

But Python developers rarely write while loops with counters to loop a fixed number of times; they use a for loop.

ans = None
for _ in range(10):
    x = int(raw_input('Please enter an integer: '))
    if x % 2 != 0: 
        if ans is None or ans < x:
            ans = x

if ans is not None:
    print 'the largest odd number entered was ', ans
else:
    print 'All numbers were even'

Minutiae

  • Python2.x's End Of Life (EOL) is January 1, 2020. If it is not a job/class requirement to study Python 2.x, you should learn Python 3.x
  • Follow the PEP-8 style guide when writing Python. There are many tool to help you check compliance, such as PyLint, PyChecker, PyFlakes to name a few.
  • ans is not a good variable name; it suggests "answer", but answer to what? greatest_odd_value is much more descriptive, but perhaps too verbose. odd_max would be a good balance between brevity and descriptiveness.
\$\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.