1
\$\begingroup\$

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.

\$\endgroup\$
  • 1
    \$\begingroup\$ In is_divisible(), you want to check that y != 0, or catch ZeroDivisionError exception. \$\endgroup\$ – boardrider Nov 19 '16 at 20:40
  • 1
    \$\begingroup\$ 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. \$\endgroup\$ – boardrider Nov 19 '16 at 20:45
  • 2
    \$\begingroup\$ @boardrider Please write those comments as an answer \$\endgroup\$ – Peilonrayz Nov 19 '16 at 21:46
  • \$\begingroup\$ @boardrider Thanks! Can you make, as Peilonrazy mentioned, your comments an answer with demonstrating code examples? \$\endgroup\$ – Mahmood Muhammad Nageeb Nov 19 '16 at 22:01
  • \$\begingroup\$ Any reason you're not using classes? \$\endgroup\$ – User319 Nov 20 '16 at 3:40
2
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Thank you, again! That's much better. There's a typo, 'obove' must be 'above'. What about raising a ZeroDivisionError in div instead of printing? What about putting print(current_value, '/', to_div) in the branch of if to_div != 0: so if to_div is a zero, the mathematically wrong operation wouldn't be printed? \$\endgroup\$ – Mahmood Muhammad Nageeb Nov 20 '16 at 19:54
  • 1
    \$\begingroup\$ Erratum fixed. Thanks. I used print instead of raising an exception, as (1) it's in line with your other coding style, and (2) it would allow the user to enter another (non 0) number - instead of the code exiting ungracefully. However, you're more than encouraged to implement the same within a try-except block, which is more Pythonic. \$\endgroup\$ – boardrider Nov 21 '16 at 15:19
2
\$\begingroup\$

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.

\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you! I've written is_divisible to get rid of the unnecessary fraction part where the result must be an integer. For example Python will return 2.0 as the value of 8 / 4. It's just about the output. \$\endgroup\$ – Mahmood Muhammad Nageeb Nov 20 '16 at 19:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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