patternpythonMinor
Find the median value of a list
Viewed 0 times
thevaluefindlistmedian
Problem
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:
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.
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 medianI 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.
Solution
First things first, you can remove the
If we do a quick run through we should get:
This removes the first if.
You can remove most of the code in the else:
This means that the code you have at the moment is:
If we go through the if now:
This should result in something like:
This should show you that you use
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
This means that
-
Binary operators need a space either side of them.
This should result in something like:
if len(sortedlist) == 1.If we do a quick run through we should get:
theList = [1]
conterofthelist = 1 / 2
medianpart = [sortedlist[0]]
median = 1This removes the first if.
You can remove most of the code in the else:
- The first
medianpartis 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
sumrather than your loop.
- You should move the
temp / 2out of the loop, to thereturn.
This should result in something like:
return sum(sortedlist[centerofthelist -1 : centerofthelist +1 ]) / 2.0This 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]Code Snippets
theList = [1]
conterofthelist = 1 / 2
medianpart = [sortedlist[0]]
median = 1def 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]return sum(sortedlist[centerofthelist -1 : centerofthelist +1 ]) / 2.0def 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]Context
StackExchange Code Review Q#126890, answer score: 5
Revisions (0)
No revisions yet.