Thats my first application on Python. Please help me do my code better. I try make the simple application . I don't know is I do it right and how do my code cleaner.
main.py . Entry point of my calculator where i just call run() method from conductor.py module.
import conductor
def main():
conductor.run()
if __name__ == '__main__':
main()
conductor.py . It is module where I hold methods for communication my modules , something like middleware (if I may say so)
import dialog
import operations
variables_list = []
def run():
dialog.print_operations_list()
processing()
def processing():
operation = dialog.get_operation()
result = dialog.get_result
if operation == '1':
result(operations.addition(get_many_variables()))
elif operation == '2':
result(operations.subtract(get_many_variables()))
elif operation == '3':
result(operations.multiply(get_many_variables()))
elif operation == '4':
result(operations.divide(get_many_variables()))
elif operation == '5':
result(operations.exponent(get_one_variable()))
elif operation == '6':
result(operations.square(get_one_variable()))
elif operation == '7':
result(operations.cos(get_one_variable()))
elif operation == '8':
result(operations.sin(get_one_variable()))
elif operation == '9':
result(operations.tan(get_one_variable()))
elif operation == '0':
result(operations.cot(get_one_variable()))
else:
print('Invalid operation. Please try again!')
run()
def get_one_variable():
variable = dialog.get_variables()
if variable == '0':
print("You can't write zero, because ist provide an error")
return get_one_variable()
elif variable != '':
variables_list.append(int(variable))
return variables_list
else:
print("Invalid number")
get_one_variable()
def get_many_variables():
variable = dialog.get_variables()
if variable == '0':
print("You can't write zero, because ist provide an error")
return get_many_variables()
elif variable != '':
variables_list.append(int(variable))
return get_many_variables()
else:
if len(variables_list) <= 1:
print('Invalid numbers. You must write minimum 2 numbers')
return get_many_variables()
else:
return variables_list
dialog.py This is module who provide communication with user by input or output some console messages.
def greetings():
print("Hi, I'm Calculator.\n")
def print_operations_list():
print("At this moment you can: \n "
"Additional, write 1 \n "
"Subtract, write 2\n "
"Multiply, write 3\n "
"Divide, write 4\n "
"Exponent, write 5\n "
"Square, write 6\n "
"Cosines, write 7\n "
"Sinus, write 8\n "
"Tangents, write 9\n "
"Cotangents, write 0\n ")
def get_operation():
return input('Write what do you want to do? \n')
def get_variables():
return input('Write number... \n')
def get_result(result):
print(f"Your result is: {result}")
operation.py This module consist of mathematical operation methods, and northing more
import math
def addition(variables):
result = variables[0]
for variable in variables[1:]:
result += variable
return result
def subtract(variables):
result = variables[0]
for variable in variables[1:]:
result -= variable
return result
def multiply(variables):
result = variables[0]
for variable in variables[1:]:
result *= variable
return result
def divide(variables):
result = variables[0]
for variable in variables[1:]:
result /= variable
return result
def exponent(variables):
return variables[0] ** 2
def square(variables):
return math.sqrt(variables[0])
def cos(variables):
return math.cos(variables[0])
def sin(variables):
return math.sin(variables[0])
def tan(variables):
return math.tan(variables[0])
def cot(variables):
return math.cos(variables[0]) / math.sin(variables[0])
2 Answers 2
Project structure
You divided your code in multiple modules and functions. While this is generally a good idea, I fail to understand the logic you used here. You should aim for each function to fully implement a single feature of your code.
In your case, a single feature may get split across multiple functions, possibly across multiple modules. For example, get_one_variable()
calls get_variable()
in another module. If you wanted to change how user inputs are handled (which you should, more on that later), now you have to apply changes all over your code, for something quite basic.
In fact, I'd argue a simple project like this would benefit from being in a single source file.
Naming
A lot of your module and function names are confusing or misleading. square
computes the square root, exponent
returns a square where I would expect any exponentiation, get_result
gets nothing but prints instead, the module names are confusing...
User input handling
One of the core functionalities is getting and handling user input. As such, special care should be given to:
- provide the user with information about what input is expected
- accept all valid inputs
- reject gracefully invalid inputs, give feedback and provide a way to try again
Your code fails on all 3 levels:
- Multiple issues:
- no mention if an operation requires one or multiple inputs
- no mention that only integers are accepted
- it is not documented that a blank input is expected to end a stream of multiple inputs
- For all these operations, any real number (
float
) (or positive real number for square root) is a perfectly valid input, yet only non-zero integers are accepted. This is especially absurd for trigonometric functions. - If a user input for a number can't be parsed as an
int
, the program crashes. Imagine entering a typo when entering the 50th number in a long sum. That would be infuriating.
For getting a floating point value, this simple function should work well enough:
def get_float_input():
while True:
try:
return float(input("Enter a real number:\n> "))
except ValueError:
print("Invalid input: could not parse number.")
It may need some more work for handling limited range inputs (e.g. for square root operands) or getting multiple inputs, but it's a good enough starting point.
Duplication
There is quite a lot of duplication taking place in your code, especially for listing all supported operations. If you wanted to support another operation, you would have to add it manually to both processing
and print_operations_list
, which is tedious and a source of error.
Use an iterable
(list or dictionary) of supported operations and iterate over it instead.
operation.py
There are a number of changes you could make here to help with readability.
variables
This is not a good name for any collection of values/parameters. Let's just make it values
or numbers
.
Functions that take multiple arguments
Instead of using start = values[0]
and then doing operations on the rest, you can use tuple unpacking. This can be done either in the function:
def addition(values):
start, *values = values
# rest of code
values = (1, 2, 3)
addition(values)
Or in the function signature
def addition(start, *values):
# rest of code
values = (1, 2, 3)
# you have to unpack it in the function call
# for this to work properly
addition(*values)
Using builtins and/or reduce
For addition
, you can just feed your values to sum
:
def addition(values):
return sum(values)
Which means that really addition
could just be an alias for sum
:
addition = sum
addition((1, 2, 3))
6
With other functions that take multiple inputs, you can use functools.reduce
and equivalent functions from the operator
module:
from operator import mul, sub, truediv as div
from functools import reduce
def multiply(start, *values):
return reduce(mul, values, start)
def subtract(start, *values):
return reduce(sub, values, start)
def divide(start, *values):
return reduce(div, values, start)
Where reduce
takes the following arguments:
- A two argument function
- An iterable of values
- A starting value
Functions that take one argument
Don't take an array of values, take a single value. Otherwise calling the function will raise some baffling errors that your value is not able to be indexed:
def exponent(values):
return values[0] ** 2
# I would expect I can do
exponent(2)
TypeError: 'int' object is not subscriptable
So instead, change the signature to more accurately reflect what you want.
exponent
and square
The names exponent
and square
would make me think that they do almost the same thing, square
would be a special case of exponent
.
Keep exponent
, but make square
into square_root
:
# Let's also add a default to exponent, in case a user
# wants a different power
def exponent(value, power=2):
return value ** power
def square_root(value):
return math.sqrt(value)
()
afterdialog.get_result
\$\endgroup\$