HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Reinventing the basic calculator functions

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thereinventingfunctionscalculatorbasic

Problem

I decided to reinvent the basic calculator functions using only:

-
the set of positive integers

-
greater than, less than, and equality properties

-
increment and decrement

(in other words, the counting on fingers method)

def is_positive_int(x):
    if type(x) == int and x >= 0:
        return True
    else:
        return False

def add(x,y):
    if is_positive_int(x) and is_positive_int(y):
        for i in range(y):
            x += 1
        return x
    else: 
        return False

def multiply(x,y):
    if is_positive_int(y) and is_positive_int(y):
        m = 0
        for i in range(y):
            m = add(m,x)
        return m
    else: 
        return False

def power(x,y):
    """ x to the y power """
    if is_positive_int(x) and is_positive_int(y):
        p = 1
        for i in range(y):
            p = multiply(p,x)
        return p
    else: 
        return False

def subtract(x,y):
    """ x - y """
    if is_positive_int(x) and is_positive_int(y) and x > y:
        for i in range(y):
            x -= 1
        return x
    else: 
        return False

def divide(x,y):
    """ x / y """
    if is_positive_int(x) and is_positive_int(y):
        floor = 0 
        remainder = 0
        for i in range(x):
            remainder += 1
            if remainder == y:
                floor += 1
                remainder = 0
        return {'floor':floor, 'modulus':remainder}
    else: 
        return False

def floor(x,y):
    """ a // b"""
    return divide(x,y)['floor']

def modulus(x,y):
    """ x % y"""
    return divide(x,y)['modulus']

Solution

Let's say you have this situation:

if ...:
    ...
    return ...
else:
    return ...


What if the if condition is True? Some statements will be executed, and then the function will return. In other words, if the condition is True, execution never leaves the if block. Therefore, you really don't need the else. Just put the return directly under the if:

if ...:
    ...
    return ...
return ...


It is generally a bad idea to use the pattern type(...) == .... Usually, we use isinstance(..., ...). That way, subclasses are allowed. Of course, you might keep it if you don't want subclasses to be allowed, but I don't see a reason to restrict the user. I'm assuming you want only integers because only integers can be passed to range, but subclasses of integers can indeed also be passed. For example, booleans are integers. range(False, True) is perfectly valid. Also, True + False + False is valid, so it won't mess up your arithmetic. That makes your is_positive_int function look like this:

def is_positive_int(x):
    if isinstance(x, int) and x >= 0:
        return True
    return False


But what do you see? If a condition is True, we return True. If it is False, we return False. In other words, we are really just returning the condition, so why not do it like this:

def is_positive_int(x):
    return isinstance(x, int) and x >= 0


I see a very similar pattern in all of your functions:

if is_positive_int(x) and is_positive_int(y):
    ...
else:
    return False


First of all, we can remember what I mentioned earlier about not leaving the if block unless the condition is False. Therefore, I would actually switch the if and else:

if not is_positive_int(x) or not is_positive_int(y):
    return False
...


We remove a level of nesting that way. The problem is that we use the same code in a bunch of different functions, so we can make a decorator. A decorator is a function which, when given another function (a) to wrap, returns a wrapper function (b) for a. b overrides a and does its special thing, but it usually calls a later. This is my idea:

def guarantee_positive(function):
    # Each function takes two arguments
    def wrapper(x, y):
        if is_positive_int(x) and is_positive_int(y):
            # Deal with it normally
            return function(x, y)
        return False
    # Now wrapper(x, y) will be called instead of function(x, y)
    return wrapper


To use it, add @guarantee_positive before each function definition. For example:

@guarantee_positive
def divide(x, y):
    floor = 0
    remainder = 0
    for i in range(x):
        remainder += 1
        if remainder == y:
            floor += 1
            remainder = 0
    return {'floor': floor, 'modulus': remainder}


That makes for a much cleaner function. Of course, subtract has an extra condition, but I'll get to that later.

With all that about function decorators, why do you need them both to be positive? I can see why you might want y to be positive: because range doesn't work the same with negative numbers, but you aren't using i most of the time, so just use the absolute value of y. I'll give an example:

def multiply(x, y):
    m = 0
    function = add if y > 0 else subtract
    for _ in range(abs(y)):
        m = function(m, x)
    return m


It doesn't require a decorator, and it is capable of more. You can now use it with any combination of negative and positive:

-3,  3 -> -9
-3, -3 ->  9
 3, -3 -> -9
 3,  3 ->  9


Your add function would be like this:

def add(x, y):
    # Add 1 if y is positive; subtract 1 if it is negative
    y_add = (1 if y >= 0 else -1)
    for _ in range(abs(y)):
        x += y_add
    return x


Again, it's shorter than the original, and it does more.

There are some functions where it isn't quite that easy. For example, power() would be difficult to implement a negative y (x doesn't need to be positive even with the current implementation), but most of the others don't need the decorator. I won't tell you all of them, but I have given examples of the more common hangups.

Back to the decorator, I think it is better to raise an exception when something is wrong than to return False. After all, they are called exceptions for a reason. Normally, the function does one thing, but there are exceptions (or there should be).

I see a lot of for i in range(y) when i isn't used. The common way of signifying that i is just a place holder is using _ as the variable name.

It looks like you cheated with divide() :) You don't actually have division, you have divmod. I think a name similar to that would be more appropriate. another thing is that you return a dictionary. I prefer something closer to how the built-in function does it:

return floor, remainder


That way, you can do:

floor, remainder = divide(...)


Instead o

Code Snippets

if ...:
    ...
    return ...
else:
    return ...
if ...:
    ...
    return ...
return ...
def is_positive_int(x):
    if isinstance(x, int) and x >= 0:
        return True
    return False
def is_positive_int(x):
    return isinstance(x, int) and x >= 0
if is_positive_int(x) and is_positive_int(y):
    ...
else:
    return False

Context

StackExchange Code Review Q#139032, answer score: 9

Revisions (0)

No revisions yet.