Advanced Python Calculator using OOP

Question

I've been into computer science fundamentals (proper data structures/algorithms/etc) including Object Oriented Programming. Therefore, I decided to reformat my old traditional Python Calculator into an Object Oriented one.

import os

#Pre-Defined
def check_int(num):
    try:
        num = int(num)
        return True
    except:
        return False

class User:
    def __init__(self, username) -> None:
        self.name = username
        Calculator.main_page(username)

class Calculator:
    def __init__(self) -> None:
        pass

    def main_page(username):
        print(f'Welcome to The Python Calculator {username}!')
        print('')
        print('Please type the number of the operation you wish to do:')
        print('1 -> Addition')
        print('2 -> Substraction')
        print('3 -> Multiplication')
        #Operation Selection
        user_input = input('')
        while check_int(user_input) != True:
            print('Error! Please give a valid response!')
            user_input = input('')
        user_input = int(user_input)

        #Interfaces       
        if user_input == 1: #ADDITION
            print("Please type in the numbers you wish to add, type x when complete!")
            nums = []
            while True: #Wait for x input
                number_in = input('')
                if number_in.lower() == "x": #Check for x
                    break
                while check_int(number_in) != True: #Check Int
                    print('Number not registered, please type a valid number!')
                    number_in = input('')
                number_in = int(number_in)
                nums.append(number_in)

            print(f'Your results are: {Calculator.addition(nums)}')
            Calculator.end_screen(username)
        
    def end_screen(username):
        print("Thank you, would you like to use the calculator again? (yes/no)")
        end_user_input = input('')
        if end_user_input.lower() == "yes":
            Calculator.main_page(username)
        elif end_user_input.lower() == "no":
            os.system('CLS' if os.name == 'nt' else 'clear')
            print('Thank you for using the calculator, have a good day!')
        else:
            print("I'm sorry, unrecognized input, you will be redirected to the main page!")
            print("")
            Calculator.main_page(username)

    #Processes
    def addition(data):
        res = sum(data)
        return res

#Main Driver
if __name__ == '__main__':
    os.system('CLS' if os.name == 'nt' else 'clear')
    
    #Input username and instantiate
    username = input('Please type your name: ')
    os.system('CLS' if os.name == 'nt' else 'clear')
    user = User(username)

So far I have only recreated the addition part yet, but I would like to know if the algorithm/code I've written is considered optimal/efficient. I would love to get some feedbacks about what could I improve in general.

Answer 1

conventional predicate name

def check_int(num):

Prefer is_int.

    try:
        num = int(num)
        return True
    except:
        return False

Avoid bare except, as it interferes with properly handling important exceptions such as KeyboardInterrupt and SystemExit. Prefer except Exception:

"busy" constructor

class User:
    def __init__(self, username) -> None:
        self.name = username
        Calculator.main_page(username)

This constructor is doing too much. Worry about the "noun" aspects, where we're just filling in slots like .name. Producing the side effect of displaying a main page and looping through prompts is too much "verb" to be appropriate here.

More formally, a class maintains one or more Invariants, and the ctor is responsible for ensuring that they hold by the time it finishes executing. For a User the invariants might be it always has a non-empty .name and some positive .age attribute.

In the OP code I see no justification for introducing a User abstraction at all. It would suffice to just pass around a username. For one thing, it's impossible to write a non-interactive unit test for the OP code, since we can't even get an instance to play with.

    def __init__(self, username) -> None:

Thank you for the "we're evaluating for side effects" annotation. It would be helpful to also point out that we expect a string:

    def __init__(self, username: str) -> None:

not object oriented

class Calculator:
    def __init__(self) -> None:
        pass

An empty ctor suggests there's "no there, there". We have no object attributes -- we're not modeling some concept.

It would make more sense to just have a calculator.py module, containing a pair of {main, end} page functions and maybe an addition function. Though that last one is merely an alias of the builtin sum(), so I'm not seeing any value add and would simply delete it.

Both of the page functions take a username argument, so I guess you could make self.username an attribute of the Calculator object. In any event it would make more sense than the OP code's classes.

conventional signature

    def main_page(username):

That's a very odd signature for a class method. It turns out the call site is Calculator.main_page(username) so it "works".

What we'd expect to see is def main_page(self):

Or prepend a @staticmethod decorator to the OP code.

It really looks like this class method would prefer to simply be a module function.

a boolean result is already a boolean expression

        while check_int(user_input) != True:

Prefer while not check_int(user_input):

nit, typo: "Substraction"

diagnostic message

Certainly this is an error message:

            print('Error! Please give a valid response!')

But it's not a diagnostic error message. It says "you lose!", but it doesn't explain how to win.

Point out that the user should type in an integer. Additionally, when given a "big" integer we should probably explain that there's only a handful of valid choices.

forgiveness vs permission

        while check_int(user_input) ...
            ...
        user_input = int(user_input)

Rather than introducing a helper predicate, consider moving that try / except up into this loop. Just convert to integer always, and if it didn't work out, fine, we can loop again until it does.

enum

        if user_input == 1: #ADDITION

This would be better phrased as if user_input == Operation.ADDITION. So create such a class.

loop termination

            while True:  # Wait for x input
                    ...
                    break

Certainly this works. But consider adopting a while number_in.lower() != "x": approach, similar to how you looped above until finding a valid integer.

separation of concerns

This is a nice little sub-problem that might be a good fit for a generator.

from collections.abc import Generator

def get_numbers(prompt: str = "Please type in the numbers you wish to add, type x when complete.") -> Generator[int, None, None]:
    print(prompt)
    while True:
        response = input("")
        if response == "x":
            return
        try:
            yield int(response)
        except ValueError:
            print("Number not registered, please type a valid number, or x to exit.")

...

def main_page(username: str) -> None:
    ...
    nums = list(get_numbers())
    ...

That way the calling code can operate at a higher level of abstraction. It says "gimme a bunch of numbers", and lets the helper worry about all those nitty gritty details.

BTW, feel free to substitute float() for int().

stack overflow

    def end_screen(username):
        print("Thank you, would you like to use the calculator again? (yes/no)")
        end_user_input = input('')
        if end_user_input.lower() == "yes":
            Calculator.main_page(username)

If we do this repeatedly, it will eventually blow the stack. We're keeping the "end" stack frame, and pushing another "main" stack frame.

Prefer to combine these into a single function, and use while for looping.

extract helper

            os.system('CLS' if os.name == 'nt' else 'clear')
    ...
    os.system('CLS' if os.name == 'nt' else 'clear')    
    ...
    os.system('CLS' if os.name == 'nt' else 'clear')

By the time you've pasted that fragment in for the third time, the code is trying to tell you something. It wants you to def clear_screen():

---

Summary:

Introduce a class abstraction only where it simplifies the calling code. Otherwise, stick with good old functions.

Prefer while loops over infinite recursion.