3
\$\begingroup\$

After reviews of my previous codes 1st, 2nd learned many new things. So with those in mind, this is my solution for another question.

Question: Write a program that ask user for list size and values and find largest and smallest no, even no, odd no?

Expected output: For user defined list,

  1. [13, 7, 19, 21, 3, 4, 27, 47, 50, 11] -> 50, 3, 50, 4, 47, 3

  2. [2, 4, 6, 8, 10] -> 10, 2, 10, 2, None, None

  3. [1, 3, 5, 7, 9] -> 9, 1, None, None, 9, 1

Format: largest_no, smallest_no, largest_even, smallest_even, largest_odd, smallest_odd

My solution:

def largest_and_smallest(given_list):
    """Extracts required numbers from given_list.

    >>> largest_and_smallest([13, 7, 19, 21, 3, 4, 27, 47, 50, 11])
    (50, 3, 50, 4, 47, 3)

    >>> largest_and_smallest([2, 4, 6, 8, 10])
    (10, 2, 10, 2, None, None)

    >>> largest_and_smallest([1, 3, 5, 7, 9])
    (9, 1, None, None, 9, 1)

    """
    sorted_list = sorted(given_list)
    largest_no = max(sorted_list)
    smallest_no = min(sorted_list)

    list_of_even = [even_no for even_no in sorted_list if ((even_no % 2) == 0)]
    if len(list_of_even) == 0:
        largest_even = None
        smallest_even = None
    else:
        largest_even = max(list_of_even)
        smallest_even = min(list_of_even)

    list_of_odd = [odd_no for odd_no in sorted_list if ((odd_no % 2) != 0)]
    if len(list_of_odd) == 0:
        largest_odd = None
        smallest_odd = None
    else:
        largest_odd = max(list_of_odd)
        smallest_odd = min(list_of_odd)

    return largest_no, smallest_no, largest_even, smallest_even, largest_odd, smallest_odd


def input_handler():
    """Checks validity of input. Any mistakes causes the program to stops."""
    size = input("Enter a size of list?\n>")
    if size.strip() == "":
        exit("Program Terminated. (No Input)")
    elif size.isdigit():
        list_size = int(size)
        list_of_no = [int(input("Enter value:")) for _ in range(list_size)]  # bad input causes error.
        return list_size, list_of_no
    else:
        exit("Program Terminated. (Bad Input)")


def main():
    print("All input should be integers.")
    list_size, list_of_no = input_handler()
    l_no, s_no, l_even, s_even, l_odd, s_odd = largest_and_smallest(list_of_no)
    print()
    print(f"From provided values of list of a size {list_size}:")
    print(f"Largest no.: {l_no}, Smallest no.: {s_no}, Largest even no.: {l_even}, "
          f"Smallest even no.: {s_even}, Largest odd no.: {l_odd}, Smallest odd no.: {s_odd}")
    return


if __name__ == '__main__':
    import doctest
    doctest.testmod(main())

So, from this review I would like to know:

  1. I have used list comprehension as per suggestion in previous code, have I used that correctly? For example, I made separate list for even and odd numbers could I have done that in different way?

  2. I made a separate function to handle input is that a bad practice? What is the best way to handle inputs, that checks different corner cases?

If there is any recommendations going forward from this will help me a lot.

\$\endgroup\$
3
\$\begingroup\$

I think overall your code looks really good. There are a few changes I would make to improve readability and such.


You use no for "number" throughout. This may be fine in your context, but in general in English code it's more common to use num to mean number. This would be a simple refactoring to just rename variables and such.


When I use Python 3.x I like to take advantage of type hints so my IDE will tell me if I'm using a function/method incorrectly.

For example, using your code:

from typing import List, Tuple

def largest_and_smallest(given_list: List) -> Tuple:
    # code here

This tells your coding tools that your function should expect a List and return a Tuple. I find it makes it easier to keep code organized and to read it, and a lot of IDEs will give a warning if you use the function incorrectly.


You can check for an empty list with not instead of checking its length is zero. For example, this:

if len(list_of_even) == 0:
    largest_even = None
    smallest_even = None

Can be shortened to this:

if not list_of_even:
    largest_even = smallest_even = None

Also combining the assignments to the variables.

You can also use the same technique for empty string, e.g.:

if size.strip() == "":

Can be written as:

if not size.strip():

I find the documentation for your primary function a bit unclear:

"""Extracts required numbers from given_list.

I would recommend something more akin to:

"""Extracts the following numbers from given_list and returns a tuple containing them:
largest_num, smallest_num, largest_even, smallest_even, largest_odd, smallest_odd
Examples:
# your examples following

To address your questions:

  1. Yes, I think you have used list comprehensions well. Personally I may have extracted if ((even_no % 2) == 0) (and not it for odd numbers) to a separate function is_even(num: int) -> bool to avoid repetition. Those parentheses also are not needed, if even_no % 2 == 0 will work fine.

  2. Having a separate function to handle inputs is good practice, it makes the code simpler to understand, and you could write tests for input to handle corner cases, for example with unittest library, which I use myself and really like.

\$\endgroup\$
  • \$\begingroup\$ Thanks ++ for this through analysis, haven't learned unittest so will do that and for other suggestion will surely follow those in future. \$\endgroup\$ – Ram Chandra Giri Oct 10 '17 at 1:14
4
\$\begingroup\$
  • Your variables are long, without needing to be.
  • You don't have to define largest_even, amongst others, it'd be simpler to just return the tuple.
  • If you're targeting 3.4+, and I don't see why you wouldn't, you can use the default keyword. "New in version 3.4: The default keyword-only argument."

Leaving you simply with:

def largest_and_smallest(numbers):
    even = [n for n in numbers if (n % 2) == 0]
    odd  = [n for n in numbers if (n % 2) != 0]
    return (
        max(numbers, default=None),
        min(numbers, default=None),
        max(even, default=None),
        min(even, default=None),
        max(odd, default=None),
        min(odd, default=None)
    )

If you don't like the verbosity of max, you could use functools.partial to set default twice.

from functools import partial

def largest_and_smallest(numbers):
    min_ = partial(min, default=None)
    max_ = partial(max, default=None)
    even = [n for n in numbers if (n % 2) == 0]
    odd  = [n for n in numbers if (n % 2) != 0]
    return (
        max_(numbers),
        min_(numbers),
        max_(even),
        min_(even),
        max_(odd),
        min_(odd)
    )
\$\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.