8
\$\begingroup\$

This program takes a user input to choose which calculation method they'd like to use, then uses a separate module and the math library to perform said calculation.

I'd like some feedback as to how this looks as a Python program. Have I broken any major rules? I've been learning Python for maybe two weeks now; the last time I used it was at high school in 2017.

main.py

import math
from modules import arithmetic 
programRunning = True
# Exploration of Python's fundamentals.
# Match Case selection for calculation selection:
while(programRunning):
 print("\nPlease select the calculation that you wish to perform:")
 print("\n-------------------------------------------------------")
 print("0 -- Square root")
 print("1 -- Exponents")
 print("2 -- Cube Root")
 print("-------------------------------------------------------")
 calculationSelection = int(input("\nPlease select the calculation that you wish to perform:"))
 match calculationSelection:
 case 0:
 arithmetic.selectSquareRoot()
 case 1:
 arithmetic.selectExponent()
 case 2:
 arithmetic.selectCubeRoot()
 case _:
 print("\nERROR: Invalid Selection\n")

arithmetic.py

import math
def selectSquareRoot(): # Square Root 
 squaredNum = int(input("\nPlease enter the square number to be rooted: "))
 squareRootNum = math.sqrt(squaredNum)
 print(f"\nThe square root of {squaredNum} is {squareRootNum}.")
def selectExponent(): # Exponents
 exponentialNum = int(input("\nPlease enter the number to be raised: "))
 powerNum = int(input("\nPlease enter the power to raise the number to: "))
 raisedExponent = math.pow(exponentialNum, powerNum)
 print(f"\n{exponentialNum} to the power of {powerNum} is {raisedExponent}.")
def selectCubeRoot(): # Cube Root
 cubedNum = int(input("\nPlease enter the cubed number to be rooted: "))
 cubeRootNum = math.cbrt(cubedNum)
 print(f"\nThe cube root of {cubedNum} is {cubeRootNum}.")
Chris
3,6411 gold badge7 silver badges32 bronze badges
asked Sep 13 at 1:29
\$\endgroup\$
1

4 Answers 4

10
\$\begingroup\$

In addition to the points raised in the previous answer, here are some other suggestions.

Naming

The PEP 8 style guide recommends snake_case for function and variable names. For example:

calculationSelection would be calculation_selection

selectSquareRoot would be select_square_root.

Simpler

As already suggested, in main.py, the while loop is simpler as:

while True:

Note that parentheses are commonly excluded for simple conditions like this.

Looping

It would be cleaner to allow the user to gracefully break out of the infinite while loop. You can add an option for the user to do so (such as entering 3). I will demonstrate with a complete example below.

DRY

The same message is shown twice:

print("\nPlease select the calculation that you wish to perform:")

It would be better to have 2 separate messages to the user, which I'll demonstrate below.

Comments

Expanding on the previous answer, this comment:

# Exploration of Python's fundamentals.

should be deleted because it does not describe your specific code.

PEP 8 recommends adding docstrings for the main code and functions. For example, in main.py, add this:

"""
Command-line calculator tool.
User inputs the type of calculation and the numbers to calculate.
"""

This comment has redundant use of "calculation", and there is no need' to state what is obvious from the code (that you use a match statement):

# Match Case selection for calculation selection:

This would be a more meaningful comment:

# User chooses the type of calculation:

Here is main.py with several of the above suggestions:

from modules import arithmetic 
import sys
"""
Command-line calculator tool.
User inputs the type of calculation and the numbers to calculate.
"""
# User chooses the type of calculation:
while True:
 print("\nPlease select the calculation that you wish to perform:")
 print("\n-------------------------------------------------------")
 print("0 -- Square root")
 print("1 -- Exponents")
 print("2 -- Cube Root")
 print("3 -- Exit program")
 print("-------------------------------------------------------")
 calculation_selection = int(input("\nSelect the calculation (or 3 to exit): "))
 match calculation_selection:
 case 0:
 arithmetic.selectSquareRoot()
 case 1:
 arithmetic.selectExponent()
 case 2:
 arithmetic.selectCubeRoot()
 case 3:
 sys.exit(0)
 case _:
 print("\nERROR: Invalid Selection\n")

Note that the sys module is imported to use exit to break out of the loop.

answered Sep 13 at 12:18
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Thanks for the reply! I think the docstrings have blown my mind after using them, makes a larger difference than regular comments. I hadn't come across the PEP 8 documentation before so I shall read those - I have a book on clean code and finding a convention for naming different items has been a tricky one so thank you for that. The sys exit to break out is great! I haven't used that one since I was at school so it had completely left my brain! I've added all suggestions into the code. Thank you! \$\endgroup\$ Commented Sep 13 at 23:00
  • \$\begingroup\$ @JakeIsaacHarvey: Glad to help. \$\endgroup\$ Commented Sep 13 at 23:10
  • \$\begingroup\$ A minor UX nitpick: I'd have the exit option be the first one. Then in case one adds more calculations to the menu, the change in user interface is minimal. \$\endgroup\$ Commented Sep 15 at 9:32
  • \$\begingroup\$ @TomG: That's an excellent suggestion. \$\endgroup\$ Commented Sep 15 at 10:33
9
\$\begingroup\$
  • main.py doesn't need to import math.

  • programRunning never becomes False. Do you really need a named variable to control an infinite loop? You also don't need to put the condition in parentheses in Python, like you do in C++. So this should be while True:

  • The select... functions are misnamed. They do not select anything. They perform (or do) something. Consider renaming.

  • It is easy to make selectSquareRoot and selectExponent to throw ValueError: math domain error, which terminates the program. You may want to address this.

  • Comments in arithmetic.py are noise. Docstrings would be much more useful.

Barmar
3851 silver badge9 bronze badges
answered Sep 13 at 4:28
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for the reply! Good to know main.py doesn't need to import math, I suppose that makes sense since it doesn't perform any of the math functions. I used the programRunning variable because in my C++ experience this is the case, not used to high level programming so glad to know that! I'll rename said functions to be more descriptive. I'll do some research into a math domain error and try to fix that. I'll also check what Docstrings are and then go from there! Thank you. \$\endgroup\$ Commented Sep 13 at 22:13
  • \$\begingroup\$ You wouldn’t need a programRunning variable in C++ either. \$\endgroup\$ Commented Sep 14 at 11:42
7
\$\begingroup\$

A further suggestion neither toolic nor vnp have touched on: your main.py can be imported in another Python source file. If this were done, all of the code in it would be executed. To avoid this, wrap your loop in a function and call it if __name__ equals '__main__'.

Modifying toolic's suggested code slightly:

import sys
"""
Command-line calculator tool.
User inputs the type of calculation and the numbers to calculate.
"""
def main():
 # User chooses the type of calculation:
 while True:
 print("\nPlease select the calculation that you wish to perform:")
 print("\n-------------------------------------------------------")
 print("0 -- Square root")
 print("1 -- Exponents")
 print("2 -- Cube Root")
 print("3 -- Exit program")
 print("-------------------------------------------------------")
 calculation_selection = int(input("\nSelect the calculation (or 3 to exit): "))
 
 match calculation_selection:
 case 0:
 arithmetic.selectSquareRoot()
 case 1:
 arithmetic.selectExponent()
 case 2:
 arithmetic.selectCubeRoot()
 case 3:
 sys.exit(0)
 case _:
 print("\nERROR: Invalid Selection\n")
if __name__ == '__main__':
 main()

We can now start a new file and import main without causing the main function to run unless we call main.main().

I would further:

  • Break out printing the menu into a function.
  • Handle the exception that can be thrown on invalid input when getting the option.
  • Use a list of functions rather than a match. The input option can be used as an index into this list, and on a KeyError we will know the user has not input a valid choice.
import sys
"""
Command-line calculator tool.
User inputs the type of calculation and the numbers to calculate.
"""
def print_menu():
 print("\nPlease select the calculation that you wish to perform:")
 print("\n-------------------------------------------------------")
 print("0 -- Square root")
 print("1 -- Exponents")
 print("2 -- Cube Root")
 print("3 -- Exit program")
 print("-------------------------------------------------------")
option_handlers = [
 arithmetic.selectSquareRoot,
 arithmetic.selectExponent,
 arithmetic.selectCubeRoot,
 lambda: sys.exit(0)
]
def main():
 # User chooses the type of calculation:
 while True:
 print_menu()
 try:
 calculation_selection = int(input("\nSelect the calculation (or 3 to exit): "))
 except ValueError:
 print("\nERROR: Must be numeric input\n")
 continue
 
 try:
 option_handlers[calculation_selection]()
 except IndexError:
 print("\nERROR: Invalid Selection\n")
if __name__ == '__main__':
 main()

We might combine those and handle the exceptions together, except that option_handlers[calculation_selection]() might raise ValueError as well.

answered Sep 13 at 15:48
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for the reply! I hadn't though that main could be imported as a module so I've added the part from the bottom, that's good to know. I had debated placing the menu into a function as I thought it made main.py look messy, glad that it was suggested. I'll do some reading into the option handlers as I've never come across them before! So thank you for that! I'll keep reading and working. \$\endgroup\$ Commented Sep 13 at 23:06
  • \$\begingroup\$ option_handlers isn't anything fancy. It's just a name for a list of functions. \$\endgroup\$ Commented Sep 13 at 23:35
3
\$\begingroup\$

I'll pick just one of the functions to review; my comments apply to all.

def selectSquareRoot(): # Square Root 
 squaredNum = int(input("\nPlease enter the square number to be rooted: "))
 squareRootNum = math.sqrt(squaredNum)
 print(f"\nThe square root of {squaredNum} is {squareRootNum}.")

If the input isn't readable, or the line can't be converted to an integer, then an exception will be thrown. Python's default exception handler is great for developers, but a little intimidating for ordinary users, so it's a good idea to catch both of these cases. I suggest catching EOFError up at the top level, and dealing with ValueError much nearer, because we can re-ask the user in the latter case:

def read_int(prompt):
 while True:
 try:
 return int(input(prompt + ": "))
 except ValueError:
 print("That was not an integer")

I don't see why we allow only integers as input. I think it's quite reasonable to want to operate on rational values such as 3.5. It's a simple change to use float instead of int, and it would make the program much more useful. We might even choose to accept complex numbers.

Another exception that can be raised is ValueError when we attempt square root of a negative value. It would be good to handle this so that we can provide a more user-friendly message without backtrace and allow the selection loop to continue.

def read_float(prompt):
 '''Read input until a float is entered'''
 while True:
 try:
 return float(input(prompt + ": "))
 except ValueError:
 print("That was not a real number")
def selectSquareRoot():
 '''Print square root of user's input'''
 x = read_float("Please enter the square number to be rooted")
 try:
 print(f"The square root of {x} is ±{math.sqrt(x)}.")
 except ValueError as e:
 print(f"Could not compute √{x}: {e}")

Alternatively, use cmath.sqrt() if we think users prefer imaginary roots to a domain error for negative inputs.

answered Sep 14 at 10:07
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for the reply! After some of the comments from yesterday I've begun to work with the exception handlers! Hadn't used them before so they're definitely a great thing to use. I should've specified that I was aiming for perfect squares to be rooted with this - hence the int data type only, but read_float seems interesting so I'll adjust it with your comments and see how I get on! I'm not sure if I can repost the edited code that I changed yesterday to display my altered work? Thank you for the reply too! \$\endgroup\$ Commented Sep 14 at 20:40
  • \$\begingroup\$ If you want a review of your updated code, the correct thing to do is to post a new question and include a link to this first iteration. Don't modify the code in this question, as that would make the answers confusing to new readers. \$\endgroup\$ Commented Sep 15 at 6:59

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.