A simple pocket calculator

Question

I was searching the internet for some Python exercises, and I've found a lab assignment from University of Toronto. Here's the major of it:

Question 1.
Welcome Message In the if __name__ == "__main__" block, write code that displays the following: Welcome to the calculator program. Current value: 0

Question 2.
Displaying the Current Value Write a function whose signature is display_current_value(), and which displays the current value in the calculator. In the if __name__ == "__main__" block, test this function by calling it and observing the output.

Question 3.
Addition Write a function whose signature is add(to_add), and which adds to_add to the current value in the calculator, and modifies the current value accordingly. In the if __name__ == "__main__" block, test the function add by calling it, as well as by calling display_current_value(). Hint: when modifying global variables from within functions, remember to declare them as global.

Question 4.
Multiplication Write a function whose signature is mult(to_mult), and which multiplies the current value in the calculator by to_mult, and modifies the current value accordingly. In the if __name__ == "__main__" block, test the function.

Question 5.
Division Write a function whose signature is div(to_div), and which divides the current value in the calculator by to_div, and modifies the current value accordingly. In the if __name__ == "__main__" block, test the function. What values of to_div might cause problems? Try them to see what happens.

Question 6.
Memory and Recall Pocket calculators usually have a memory and a recall button. The memory button saves the current value and the recall button restores the saved value. Implement this functionality.

Question 7.
Undo Implement a function that simulates the Undo button: the function restores the previous value that appeared on the screen before the current one.

Here's my solution:

current_value = 0
memory = current_value
memo = [current_value]

def is_divisible(x, y):
    """Return whether x is divisible by y. """
    return x % y == 0


def update_list(lst, value):
    """Updates a list with an item. """
    lst.append(value)


def display_current_value():
    """Prints the current value of the calculator. """
    print('Current value:', current_value)


def add(to_add):
    """Adds a number to the current value of the calcalutor. """
    global current_value

    current_value += to_add

    update_list(memo, current_value)


def mult(to_mult):
    """Multiplies the current value of the calcalutor by a number. """
    global current_value

    current_value *= to_mult

    update_list(memo, current_value)


def div(to_div):
    """Divides the current value of the calcalutor by a number. """
    global current_value

    if is_divisible(current_value, to_div):
        current_value //= to_div
    else:
        current_value /= to_div

    update_list(memo, current_value)


def store():
    """Stores the current value. """
    global memory

    memory = current_value


def recall():
    """Recalls the saved value. """
    return memory


def undo():
    """Restores the previous value. """
    global memo

    if len(memo) >= 2:
        global current_value

        current_value = memo[-2]

        memo.pop(-2)


def main():
    print('Welcome to the calculator program.' )
    display_current_value()

    add(6)
    display_current_value()

    mult(2)
    display_current_value()

    div(3)
    display_current_value()

    for iteration in range(3):
        undo()
        display_current_value()


if __name__ == '__main__':
    main()

Is it possible to implement undo without dealing with other functions such as add? Is the code well-documented?

Please point out any bad practice or any bug I've made. Please point out how somethings can be done better. And please note that I'm a beginner and a hobbyist.

Answer 1

I was asked to rewrite my comments as an answer.

• You want to check that your divisor != 0, or catch ZeroDivisionError exception.
• You may want to rethink your liberal use of globals. You can have the same behaviour, and a more robust code, if you send the value in the global to the function, and assign the result of the function.

Here's the code with the above implemented:

$ cat calc.py
def is_divisible(x, y):
    """Return whether x is divisible by y. """
    return x % y == 0


def update_list(lst, value):
    """Updates a list with an item. """
    lst.append(value)
    return lst


def display_current_value(current_value):
    """Prints the current value of the calculator. """
    print('Current value:', current_value)


def add(to_add, current_value, memo):
    """Adds a number to the current value of the calcalutor. """

    print (current_value,"+",to_add)
    current_value += to_add

    memo = update_list(memo, current_value)
    return current_value


def mult(to_mult, current_value, memo):
    """Multiplies the current value of the calcalutor by a number. """

    print (current_value,"*",to_mult)
    current_value *= to_mult

    memo = update_list(memo, current_value)
    return current_value


def div(to_div, current_value, memo):
    """Divides the current value of the calcalutor by a number. """

    print (current_value,"/",to_div)
    if to_div != 0:
        if is_divisible(current_value, to_div):
            current_value //= to_div
        else:
            current_value /= to_div
    else:
        print ("you tried division by 0")


    memo = update_list(memo, current_value)
    return current_value


def undo(current_value, memo):
    """Restores the previous value. """

    print("undo")
    if len(memo) >= 2:

        memo.pop()
        current_value = memo[-1]

    return current_value, memo


def main(current_value, memory, memo):
    print('Welcome to the calculator program.' )
    display_current_value(current_value)

    current_value = add(6, current_value, memo)
    display_current_value(current_value)

    current_value = mult(2, current_value, memo)
    display_current_value(current_value)

    current_value = div(3, current_value, memo)
    display_current_value(current_value)

    for iteration in range(3):
        # print("memo",memo)
        current_value, memo = undo(current_value, memo)
        display_current_value(current_value)


if __name__ == '__main__':
    current_value = 0
    memory = current_value
    memo = [current_value]
    main(current_value, memory, memo)

$ python3 calc.py
Welcome to the calculator program.
Current value: 0
0 + 6
Current value: 6
6 * 2
Current value: 12
12 / 3
Current value: 4
undo
Current value: 12
undo
Current value: 6
undo
Current value: 0

Note: for completion sake, you may want to add a function for substruction.

Answer 2

With your three globals, it's not clear what memo is from looking at it. Try previous_values instead. Additionally, populating the list with memory at the beginning makes me think they're related, which I'm not sure is actually the case.

boardrider suggests passing your globals as arguments to functions. I agree that this would be a good idea. Another approach would to just wrap it all in a Calculator class. Classes are good for managing state.

Your undo function is a bit weird:

def undo():
    """Restores the previous value. """
    global memo

    if len(memo) >= 2:
        global current_value

        current_value = memo[-2]

        memo.pop(-2)

The previous current_value remains in memory, which makes me not trust it. What happens if you repeatedly undo, then operate? I haven't stepped through to verify, but I suspect a bug.

I'd try this instead:

def undo():
    if len(memo) >= 2:
        global current_value
        memo.pop()
        current_value = memo[-1]

This would maintain the invariant that the current value is always the last item in the list.

My final comment I am much less unsure of, and is probably down to a matter of personal taste:

I'm not sure about this part of your division:

if is_divisible(current_value, to_div):
    current_value //= to_div
else:
    current_value /= to_div

Is this just to help prevent floating-point rounding errors? I think this is overcomplicating things without fully addressing the problem. What I mean is that I could contrive to multiply together two floating-point numbers that have the same value of integers, and it would yield another floating-point int-like number, which is what it looks like you're trying to avoid here. I just don't see value in worrying about this in one specific case.

As another idea, you could do something like if current_value == int(current_value): current_value = int(current_value) after each op.