Disclaimer: You asked me to be harsh, so I'm not holding back as much. I tried to though...
Welcome to Code Review! Your python code has been put through my scrutiny and I have some concerns and suggestions for improvement. Any criticisms and scrutiny are to be taken at their face value unless stated otherwise, and may include extremely harsh criticisms. I tried to avoid being overly harsh, but there are cases where I have to be harsh because of the severity of the mistakes or issues.
MISSING MODULES AND CODE: clear
You state you wrote your own code for clear
. Because I don't have your clear()
code I can't review it, and to make sure things work I need to yank it out of the code for review purposes. So, clear()
won't exist in the final code examples.
The missing code for clear
aside, this is the review I have written so far:
from clear import *
: BAD FORM!
Wildcard imports are bad form! PEP8, the Style Guide for Python, explicitly discourages using wildcard imports.
To quote PEP8:
Wildcard imports (from import *) should be avoided, as they make it unclear which names are present in the namespace, confusing both readers and many automated tools. There is one defensible use case for a wildcard import, which is to republish an internal interface as part of a public API (for example, overwriting a pure Python implementation of an interface with the definitions from an optional accelerator module and exactly which definitions will be overwritten isn't known in advance).
You are not taking a private internal interface and republishing it for public API, so that doesn't apply. Replace any functions you're using from clear.*
with clear.FUNCTION
instead. This is so we know what module the specific bit(s) work with.
Alternatively, you can import the specific function with:
from clear import clear
... which would let you do clear()
calls like you have in your code. I have gone with this for the code example at the end.
Bad form: declaring modules inside your main() function
This isn't C, C++, or Java. You don't need to declare your functions within main()
. In fact, unless you are working with class
es and defining functions for a class, you should not be declaring your functions within a class. Take your mean
, median
, and range_
functions and declare them individually outside the main
class. Your general code structure would then look like this:
def mean():
...
def median():
...
def range_():
...
def main():
...
This is completely acceptable and in fact the proper way to do these types of declarations.
Unused Function: end()
You create the function end
but don't use it. Either leave it out entirely because its code is already written into the code that executes after main
, or replace the code after the main()
call with end()
. However, I would suggest that you do the second of these, rather than call (end) just to re-call main()
later.
Use escape characters instead of triple-apostrophe strings when working with New Lines
You have this type of print statement at least twice:
print('''
Great choice! Choose again or type stop the sequence.''')
Functionally, this works, but from a code readability perspective, it's ugly. Replace this with escape-charactered strings instead, like:
print("\nGreat choice! Chose again or type stop the sequence.")
This reads better to those of us doing the reviews.
Use string formatting rather than appending strings to other strings
Here's an example of what I'm talking about. You do this in many different places in the code:
return 'Mean: ' + str(sum(list_one)/len(list_one))
This works, but... string appending is not the nicest thing in the world. The proper way to do this nowadays, because you'll undoubtedly in the future have longer strings with multiple things needing inserted into it instead of just one thing at the end, is to use a format string. This'd be such as this:
return 'Mean: {}'.format(sum(list_one) / len(list_one))
This also reads slightly nicer because string casting just takes up extra characters and space. Also you'll note that with this I added some spaces around that /
- this makes math more readable, actually, to those of us doing reviews.
Use one print line instead of multiple
print
, when combined with strings and line endings and format
capable strings can actually post many things at once!
So let's take my last suggestion and turn your print
calls for mean, median, and range into one statement:
print("{}\n{}\n{}".format(mean(), median(), range_()))
We get to save some typing here, too, as well.
Consider including #!
(shebang) syntax at the first line
This way, we can call your Python code directly in command lines such as the Linux shell with ./file.py
and execute the code that way (if execute bit is set). This is not a requirement, but it's nice to have so I don't have to type python3 ./filename.py
or such to run your script.
I apologize if I was overly harsh, but you said not to hold back, and I gave you a warning early on. I tried to be as non-harsh as I could.
With all my suggestions above, you get something like this:
#Mean, median, and range
def mean():
return 'Mean: {}'.format(sum(list_one) / len(list_one))
def median():
length = len(list_one)
if length % 2 == 0:
return 'Median: {}'.format(sum(list_one[length // 2 - 1:length // 2 + 1]) / 2)
else:
return 'Median: {}'.format(list_one[length // 2])
def range_():
return 'Range: {}'.format(list_one[-1] - list_one[0])
def main():
list_one = []
while True:
q = input('Type a number or type stop at any time: ').lower()
if q.isnumeric():
list_one.append(int(q))
print("\nGreat choice! Choose again or type stop the sequence.")
elif q == 'stop':
list_one.sort(key=int)
print("\nAnd the values are...")
print("{}\n{}\n{}".format(mean(), median(), range_()))
break
else:
print("\nThat\'s not a number or stop! Try again brudda man.")
main()
while True:
q = input("\nType anything and and press enter to repeat or press only \'Enter\' to exit. ")
if len(q) >= 1:
main()
else:
exit()
However, we need the code for your clear
module to be able to do a full and complete review, so this review is only partial until such time you provide us with that code. Note that I yanked all clear()
references in the interim as well.
clear
. \$\endgroup\$