Find the median value of a list

Question

I am taking an online training course on Python, and am at the following question:

Write a function called median that takes a list as an input and returns the median value of the list. For example: median([1,1,2]) should return 1. The list can be of any size and the numbers are not guaranteed to be in any particular order. If the list contains an even number of elements, your function should return the average of the middle two.

My code seems correct and passes the tests:

def median(thelist):
    median = 0
    sortedlist = sorted(thelist)
    lengthofthelist = len(sortedlist)
    centerofthelist = lengthofthelist / 2
    if len(sortedlist) == 1:
        for value in sortedlist:
            median += value
        return median

    elif len(sortedlist) % 2 == 0:
        temp = 0.0
        medianparties = []
        medianparties = sortedlist[centerofthelist -1 : centerofthelist +1 ]
        for value in medianparties:
            temp += value
            median = temp / 2
        return median

    else:
        medianpart = []
        medianpart = [sortedlist[centerofthelist]]
        for value in medianpart:
            median = value
        return median

I am (pretty) sure that, despite passing the tests, the code is neither nice, efficient, nor Pythonic. I am wondering if the community may highlight what I did wrong and how I can improve it.

Answer 1

First things first, you can remove the if len(sortedlist) == 1. If we do a quick run through we should get:

theList = [1]
conterofthelist = 1 / 2
medianpart = [sortedlist[0]]
median = 1

This removes the first if.
You can remove most of the code in the else:

• The first medianpart is immediately overridden.
• Median part could be just a number, not a list.
• You don't need to loop through medianpart.
• You can just return sortedlist[centerofthelist]

This means that the code you have at the moment is:

def median(thelist):
    median = 0
    sortedlist = sorted(thelist)
    lengthofthelist = len(sortedlist)
    centerofthelist = lengthofthelist / 2
    if len(sortedlist) % 2 == 0:
        temp = 0.0
        medianparties = []
        medianparties = sortedlist[centerofthelist -1 : centerofthelist +1 ]
        for value in medianparties:
            temp += value
            median = temp / 2
        return median
    else:
        return sortedlist[centerofthelist]

If we go through the if now:

• You don't need the first medianparties.
• You can use sum rather than your loop.
• You should move the temp / 2 out of the loop, to the return.

This should result in something like:

return sum(sortedlist[centerofthelist -1 : centerofthelist +1 ]) / 2.0

This should show you that you use lengthofthelist twice, and median never. You could probably remove these.

---

Python has a style guide called PEP8, it's a fancy name I know. It gives a few rules that you should probably follow:

• Variable names should be lower_snake_case.
  This means that sortedlist should be sorted_list and theList should be the_list.

• Binary operators need a space either side of them. centerofthelist - 1.

• List slices : have to have no spaces either side.
• Use good variable names.

This should result in something like:

def median(numbers):
    numbers = sorted(numbers)
    center = len(numbers) / 2
    if len(numbers) % 2 == 0:
        return sum(numbers[center - 1:center + 1]) / 2.0
    else:
        return numbers[center]

Answer 2

Your naming is much better than that used by most people who come here. The names are very clear so I can see exactly what logic you are using. My one suggestion is to use _ between different words; for example, length_of_the_list. It isn't necessary, but it's a little better in my opinion. I, actually, would just say length. There isn't anything else here that has a length.

You defined lengthofthelist, but you don't appear to be using it. You keep re-finding the length for each if and elif. Use the variables that you create.

Your first if makes sure that sortedlist has only one item, but then iterates through it. If it has just one item, the sum of all of its items will be just that one item. Therefore, you can say return sortedlist[0].

You define medianparties as an empty list, but then you re-define it to something else. The first definition is useless. The second, in fact, is also useless because you can put it right into the for loop: for value in sortedlist[...]:

In the elif you define median within the loop, but it doesn't need to be defined until the end. In fact, you can just take out its definition and change your return to return temp / 2. Well, really, use the sum() function instead of finding out the sum yourself.

Your else has a couple interesting things. As I mentioned above, you shouldn't define a variable if all you are going to do is redefine it. Therefore, the first definition of medianpart is useless. You then define medianpart as a list with one item: sortedlist[centerofthelist]. After that, you go through each item in that list (which is just one item), and assign median to it. If all you are doing with a list is assigning a variable to each item, that variable will be the last item in the last once you finish. You can just assign it to the last item with median = medianpart[-1], but what is the last item? Of course, it is the only item: sortedlist[centerofthelist]. Since you already know what the item is, why not assign it directly without defining medianpart at all? In fact, you don't even need a variable defined anyway. You can change your whole else block to this:

else:
    return sortedlist[centerofthelist]

The full program:

def median(thelist):
    sorted_list = sorted(thelist)
    length = len(sorted_list)
    center = length // 2

    if length == 1:
        return sorted_list[0]

    elif length % 2 == 0:
        return sum(sorted_list[center - 1: center + 1]) / 2.0

    else:
        return sorted_list[center]