HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Taking a list of numbers and finding the two largest

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thenumberslargesttwofindingandlisttaking

Problem

The code ive written takes the user entered numbers and sorts them into a list and finds the two biggest numbers, im looking to make the code less verbose. any help/advice is greatly appreciated.

def printTwoLargest():
    numList = []
    num = 'holder'
    x=0
    y=0
    while num != '':
        num = input('Please enter a number: ')
        if num != '':
            numList.append(float(Decimal(num)))
        else:
            break
    numList.sort(reverse=True)
    for i in range(len(numList)):
        if numList[i]>x:
            x = numList[i]
        elif numList[i]>y:
            y = numList[i]
        else:
            pass
    if x>0 and y>0:
        print('The largest number is:',x)
        print('The second largest number is:',y)
    elif x>0:
        print('The largest number is:',x)
    else:
        print('No positive numbers were entered.')

Solution

You should change your function to take a list and return the two largest of it.
And so you should move the creation of numList out into it's own function.
You should also move the printing out of the function too, and instead return a list of the two largest.
This should spilt your code up to achieve:

def getNumbers():
    numList = []
    num = 'holder'
    while num != '':
        num = input('Please enter a number: ')
        if num != '':
            numList.append(float(Decimal(num)))
        else:
            break
    return numList

def twoLargest(numbers):
    x=0
    y=0
    numbers.sort(reverse=True)
    for i in range(len(numbers)):
        if numbers[i] > x:
            x = numbers[i]
        elif numbers[i] > y:
            y = numbers[i]

    if y > 0:
        return x, y
    if x > 0:
        return x,
    return (,)

if __name__ == '__main__':
    numbers = getNumbers()
    largest = twoLargest(numbers)
    print(largest)


However you can improve getNumbers, rather than checking to break from the loop twice, you can instead check once.
This removes the need for num = 'holder', and can reduce the amount of lines needed.

def getNumbers():
    numList = []
    while True:
        num = input('Please enter a number: ')
        if num == '':
            break
        numList.append(float(num))
    return numList


However it can further be simplified by making it a generator function.
To do this you can just yield the values, rather than numList.append them.

def getNumbers():
    while True:
        num = input('Please enter a number: ')
        if num == '':
            break
        yield float(num)


Moving onto twoLargest, it doesn't work with a generator, as you don't cast numList to a list.
You also use sort the list in descending order, so rather than using a loop to find the largest two, you should instead use slicing.
This can get you:

def twoLargest(numbers):
    return sorted(numbers, reverse=True)[:2]


Going forward, you can make an 'n' largest function.
But rather than using sorted you can use; heapq.nlargest, and max.
Making the interface as standard as possible you can get:

from heapq import nlargest

def nLargest(numbers, n=1, key=None, sort=False):
    kwargs = {'key': key}
    if key is None:
        del kwargs['key']
    if n == 1:
        try:
            return [max(numbers, **kwargs)]
        except ValueError: # if passed an empty iterable
            return []
    if sort:
        return sorted(numbers, reverse=True, **kwargs)[:n]
    else:
        return nlargest(n, numbers, **kwargs)


The reason for the three methods is described under nlargest:


The latter two functions perform best for smaller values of n.
For larger values, it is more efficient to use the sorted() function.
Also, when n==1, it is more efficient to use the built-in min() and max() functions.
If repeated usage of these functions is required, consider turning the iterable into an actual heap.

Code Snippets

def getNumbers():
    numList = []
    num = 'holder'
    while num != '':
        num = input('Please enter a number: ')
        if num != '':
            numList.append(float(Decimal(num)))
        else:
            break
    return numList

def twoLargest(numbers):
    x=0
    y=0
    numbers.sort(reverse=True)
    for i in range(len(numbers)):
        if numbers[i] > x:
            x = numbers[i]
        elif numbers[i] > y:
            y = numbers[i]

    if y > 0:
        return x, y
    if x > 0:
        return x,
    return (,)

if __name__ == '__main__':
    numbers = getNumbers()
    largest = twoLargest(numbers)
    print(largest)
def getNumbers():
    numList = []
    while True:
        num = input('Please enter a number: ')
        if num == '':
            break
        numList.append(float(num))
    return numList
def getNumbers():
    while True:
        num = input('Please enter a number: ')
        if num == '':
            break
        yield float(num)
def twoLargest(numbers):
    return sorted(numbers, reverse=True)[:2]
from heapq import nlargest

def nLargest(numbers, n=1, key=None, sort=False):
    kwargs = {'key': key}
    if key is None:
        del kwargs['key']
    if n == 1:
        try:
            return [max(numbers, **kwargs)]
        except ValueError: # if passed an empty iterable
            return []
    if sort:
        return sorted(numbers, reverse=True, **kwargs)[:n]
    else:
        return nlargest(n, numbers, **kwargs)

Context

StackExchange Code Review Q#151491, answer score: 4

Revisions (0)

No revisions yet.