2
\$\begingroup\$

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])
\$\endgroup\$
2
  • \$\begingroup\$ Please edit the question to fix indentation. \$\endgroup\$
    – JosefZ
    Dec 17, 2022 at 20:18
  • \$\begingroup\$ You are missing () after dialog.get_result \$\endgroup\$
    – mousetail
    Dec 18, 2022 at 15:41

2 Answers 2

2
\$\begingroup\$

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:

  1. provide the user with information about what input is expected
  2. accept all valid inputs
  3. reject gracefully invalid inputs, give feedback and provide a way to try again

Your code fails on all 3 levels:

  1. 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
  2. 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.
  3. 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.

\$\endgroup\$
1
\$\begingroup\$

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:

  1. A two argument function
  2. An iterable of values
  3. 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)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.