Python binary calculator

Question

My task was to build a calculator in Python supporting binary addition etc.

To begin I define a couple of custom exceptions :

class MaximumValueException(Exception):
    pass


class MinimumValueException(Exception):
    pass

I then define the Binary class which does most of the work for me (I know that the division is not totally working yet but everything else is) :

class Binary:
    def __init__(self, application, integer: int = None):
        self.app = application
        self.binary_value = self.convert_int_to_binary(integer) if integer is not None else None

    def from_binary(self, binary: str):  # init from binary as opposed to an integer
        self.binary_value = self.allocate_bits(binary[::-1])[::-1]
        return self

    def __str__(self):
        return self.binary_value

    def __len__(self):
        return len(self.binary_value)

    def __add__(self, other):
        first = list(self.binary_value[::-1])  # flips both around to work with left to right rather than opposite
        second = list(other.binary_value[::-1])

        final_added = ""
        carry = False

        for i in range(0, len(first)):
            added_val = int(first[i]) + int(second[i])
            if carry:
                added_val = added_val + 1

            if added_val == 0 or added_val == 1:
                final_added = final_added + str(added_val)
                carry = False
            else:
                final_added = final_added + str(added_val - 2)
                carry = True

        return Binary(self.app).from_binary(final_added[::-1])

    def __sub__(self, other):
        return self + (- other)

    def __mul__(self, other):
        other_value_reversed = list(other.binary_value[::-1])
        total = Binary(self.app, 0)
        for i in range(1, len(other_value_reversed) + 1):
            binary_val = int(self.binary_value) * int(other_value_reversed[i - 1]) * int(10 ** (i - 1))
            total = total + Binary(self.app).from_binary(str(binary_val))
        return total

    def __ge__(self, other):
        return self.denary() >= other.denary()  # I know this isn't ideal but will come back around to it.

    def __le__(self, other):
        return self.denary() <= other.denary()

    def __truediv__(self, divisor):
        dividend = self.binary_value
        cur_div = ""
        answer = ""

        for i in range(0, len(dividend)):
            cur_div = cur_div + dividend[i]
            if divisor <= Binary(self.app).from_binary(cur_div):
                answer = answer + "1"
                take_away = divisor * Binary(self.app).from_binary(answer)
                cur_div = (Binary(self.app).from_binary(cur_div) - take_away).binary_value

            else:
                answer = answer + "0"

        print(answer)
        answer = Binary(self.app).from_binary(answer)
        print(answer.denary())
        return self

    def __neg__(self):
        return self.change_state()

    def denary(self):
        binary = list(self.binary_value)  # flips string
        value = 0
        if binary[0] == "1":
            binary = list(self.change_state().binary_value[::-1])
            for i in range(0, len(binary) - 1):
                value += ((2 ** i) * int(binary[i]))
            return -int(value)
        elif binary[0] == "0":
            binary = list(self.binary_value[::-1])
            for i in range(0, len(binary) - 1):
                value += ((2 ** i) * int(binary[i]))
            return int(value)

    def convert_int_to_binary(self, number):
        neg = False
        if number < 0:
            neg = True
        binary = self.positive_binary_convert(abs(number))
        if neg:
            binary = binary.change_state()
        return binary.binary_value

    def positive_binary_convert(self, integer):
        before = ""
        finished = False
        while not finished:
            divided = int(int(integer) / 2)
            remainder = int(integer) % 2
            before = before + str(remainder)
            integer = divided
            if divided == 0:
                finished = True
        before = self.allocate_bits(before)
        binary = before[::-1]  # Flips the string
        return Binary(self.app).from_binary(binary)

    def change_state(self, binary=None):  # Change from negative to positive
        if binary is not None:
            flipped = Binary(self.app).from_binary(binary).flip_bits()
        else:
            flipped = self.flip_bits()

        return flipped + Binary(self.app, 1)

    def allocate_bits(self, value):
        length = len(value)
        if length < self.app.bits:
            rem = self.app.bits - length
        else:
            rem = length % self.app.bits
        for i in range(0, rem):
            value = value + "0"
        return value

    def flip_bits(self):
        binary_list = list(self.binary_value)

        for i in range(0, len(self.binary_value)):
            binary_list[i] = "1" if binary_list[i] == "0" else "0"
        binary = "".join(binary_list)
        return Binary(self.app).from_binary(binary)

Finally I define a Application class:

class Application:
    def __init__(self, bits=8):
        self.bits = bits
        self.max_bound = (2 ** (self.bits - 1)) - 1
        self.min_bound = self.max_bound * -1

    def welcome_message(self):
        print("Welcome to the Binary Calculator!")
        print("---------------------------------")
        print("This Calculator is currently set to " + str(self.bits) + " bits.")
        print("The Maximum Bound of the calculator is " + str(self.max_bound))
        print("The Minimum Bound of the calculator is " + str(self.min_bound))

    def input_number(self, message):
        while True:
            try:
                user_input = int(input(message))
                if user_input > self.max_bound:
                    raise MaximumValueException()
                if user_input < self.min_bound:
                    raise MinimumValueException()

            except ValueError:
                print("Not an integer! Try again.")
                continue
            except MaximumValueException:
                print('Value above maximum bound of ' + str(self.max_bound))
            except MinimumValueException:
                print('Value below minimum bound of ' + str(self.min_bound))

            else:
                return user_input

    @staticmethod
    def input_sum_type():
        return str(input("What do you want to do (add, take, divide, multiply)? "))

    def input_integer(self, message):
        denary = self.input_number(message)
        binary = Binary(self, denary)
        print(binary)
        return binary

    @staticmethod
    def output_spacer():
        print("--------")

    @staticmethod
    def calculate(sum_type, first_binary, second_binary):
        if sum_type in ["add", "+"]:
            return first_binary + second_binary
        elif sum_type in ['take', '-']:
            return first_binary - second_binary
        elif sum_type in ['multiply', 'times', 'x', '*']:
            return first_binary * second_binary
        else:
            return None

    def output_calculated(self, calculated):
        print(calculated)
        self.output_spacer()
        print(calculated.denary())

    def retry(self):
        try_again = str(input("Do you want to try again? "))
        if try_again.lower() in ["yes", "y", "yea"]:
            self.main()
        else:
            print("Thanks.")

    def test(self):
        print("Passed" if (Binary(self, 5) + Binary(self, 10)).denary() == 15 else "Failed")
        print("Passed" if (Binary(self, 125) + Binary(self, 125)).denary() == 250 else "Failed")
        print("Passed" if (Binary(self, 5) - Binary(self, 10)).denary() == -5 else "Failed")
        print("Passed" if (Binary(self, 255) - Binary(self, 120)).denary() == 135 else "Failed")
        print("Passed" if (Binary(self, 4) * Binary(self, 5)).denary() == 20 else "Failed")
        print("Passed" if (Binary(self, 125) * Binary(self, 1000)).denary() == 125000 else "Failed")

    def start(self):
        self.welcome_message()
        self.main()

    def main(self):
        sum_type = self.input_sum_type()
        first_binary = self.input_integer("Enter your first Integer: ")
        second_binary = self.input_integer("Enter your Second Integer: ")
        self.output_spacer()
        calculated = self.calculate(sum_type, first_binary, second_binary)
        if calculated is not None:
            self.output_calculated(calculated)
        self.retry()

I then start the application:

Application(64).start()

Answer 1

General design comment

The fact that a binary number needs to access the application using it looks like a huge design issue.

A quick fix for this is to provide the desired numbers of bits instead - I am not sure how relevant this suggestion is but it was enough to get me started. Now, we have a Binary class with no dependencies and some tests for it:

def test(bits = 64):
    test_equal(Binary(bits, 5) + Binary(bits, 10), 15)
    test_equal(Binary(bits, 125) + Binary(bits, 125), 250)
    test_equal(Binary(bits, 5) - Binary(bits, 10), -5)
    test_equal(Binary(bits, 255) - Binary(bits, 120), 135)
    test_equal(Binary(bits, 4) * Binary(bits, 5), 20)
    test_equal(Binary(bits, 125) * Binary(bits, 1000), 125000)

Improving def __add__(self, other)

The __add__ method is interesting because it gives a good chance to see many small techniques to improve Python code.

• In Place operators: for most operations (like +) there is a in-place version so that you can write a += b instead of a = a + b.

• In Python, Booleans True and False are actually integers that evaluate to 1 and 0. Instead of: added_val = e; if carry: added_val += 1, you could write added_val = e + carry.

• Look like a native: I highly recommend the article/talk from Ned Batchelder I've linked to. Basically, pretty much everytime you write are using indices and/or range(len(...)) in Python, there is a better way to do it using various Python tools.

In our case, we are using indices to loop over 2 iterables in parallel. This could be achieved with zip.

So far, we have:

def __add__(self, other):
    first = list(self.binary_value[::-1])  # flips both around to work with left to right rather than opposite
    second = list(other.binary_value[::-1])
    assert len(first) == len(second)
    final_added = ""
    carry = False

    for i, j in zip(first, second):
        added_val = int(i) + int(j) + carry

        if added_val == 0 or added_val == 1:
            final_added += str(added_val)
            carry = False
        else:
            final_added += str(added_val - 2)
            carry = True

    return Binary(self.bits).from_binary(final_added[::-1])

Then, we could re-think the operations we are performing. For a start, we could reduce duplication by performing final_added += str(XXX) in a single place. We'd write:

        if added_val == 0 or added_val == 1:
            carry = False
        else:
            added_val -= 2
            carry = True
        final_added += str(added_val)

Because we are handling a number with is in (0, 1, 2, 3) at this stage (a sum of 3 bits), we are actually doing the equivalent of integer divisions by 2 by handling the different case. This can be written:

        assert added_val in (0, 1, 2, 3)
        carry = added_val // 2
        added_val %= 2

Even better, Python has a shortcut to get both the quotient and the remainder of a division: divmod.

       carry, added_val = divmod(added_val, 2)

• We can get rid of useless operations: the conversions to list are not required: as far as we are concerned, we just need to iterate over things: it works if they are strings.

• We can also get rid of the [::-1] trick and just use reversed instead.

• Also, we don't even need the additional variables first and second anymore.

At this stage, we have

def __add__(self, other):
    final_added = ""
    carry = False

    for i, j in zip(reversed(self.binary_value), reversed(other.binary_value)):
        carry, bit = divmod(int(i) + int(j) + carry, 2)
        final_added += str(bit)
    # TODO: Handle inputs of different length or carry overflow ?
    return Binary(self.bits).from_binary(final_added[::-1])

It looks like it can hardly get any better, doesn't it ? Well it could: PEP 8, the Style Guide for Python recommends:

do not rely on CPython's efficient implementation of in-place string concatenation for statements in the form a += b or a = a + b. This optimization is fragile even in CPython (it only works for some types) and isn't present at all in implementations that don't use refcounting. In performance sensitive parts of the library, the ''.join() form should be used instead. This will ensure that concatenation occurs in linear time across various implementations.

Improving def __mul__(self, other)

Now that we've learnt a few Python tricks and best practises, we can have a look at __mul__ and re-apply most of them. Here again, we can get rid of range(len(XXX)), use in-place operator, use reversed, remove variables:

def __mul__(self, other):
    total = Binary(self.bits, 0)
    for i, val in enumerate(reversed(other.binary_value)):
        total += Binary(self.bits).from_binary(str(int(self.binary_value) * int(val) * int(10 ** i)))
    return total

Now, something which may not be so interesting in this case but is always good to know is that the sum builtin can be use to rewrite such a expression in a more concise and more efficient way. In our case, it leads to an expression getting a bit too complicated:

def __mul__(self, other):
    return sum(
            (Binary(self.bits).from_binary(str(int(self.binary_value) * int(val) * int(10 ** i)))
                for i, val in enumerate(reversed(other.binary_value))),
            Binary(self.bits, 0))

Improving def denary(self)

Removing the duplicated logic (and introducing new variables), we could write:

def denary(self):
    first_bit = self.binary_value[0]
    if first_bit == "1":
        sign = -1
        newval = self.change_state()
    elif first_bit == "0":
        sign = 1
        newval = self
    value = 0
    binary = newval.binary_value[::-1]
    for i in range(0, len(binary) - 1):
        value += ((2 ** i) * int(binary[i]))
    return int(value) * sign

Now, it seems like the conversion int(value) is not required. Also, we could reuse our friend sum:

    return sign * sum(((2 ** i) * int(binary[i])) for i in range(0, len(binary) - 1))

Now, we also can try to get rid of the range(len(XXX)) logic but before to do so, we have to understand what is going on: this time we do not handle all elements from binary, we stop before the last one. This is quite unexpected. Actually, it is easier to understand if we look at the whole method: we have handled the first character and we want to handle the remaining ones. The special character is not the last one, it is the first.

Keeping that mind, we can re-use the techniques already described: enumerate, reversed, etc.

def denary(self):
    first_bit = self.binary_value[0]
    remaining = self.binary_value[1:]
    sign = 1
    if first_bit == "1":
        sign = -1
        remaining = self.change_state().binary_value[1:]
    return sign * sum(((2 ** i) * int(val)) for i, val in enumerate(reversed(remaining)))

Improving def allocate_bits(self, value):

We could use value += "0" but that would be still using the "in-place string concatenation" which is best to avoid when it is possible. In our case, we have a shorter and more efficient option: the mult operation on strings:

    return value + "0" * rem

Improving def flip_bits(self)

Here again, we have the range(len(XXX)) pattern but we'll handle it slightly differently. Here, it looks like we are trying to update a list as we iterate. What we are actually trying to achieve is to build a new list based on something we can iterate on. We could use for bit in self.binary_value and list.append but it is a good chance to show off list comprehension:

    binary_list = []
    for bit in self.binary_value:
        binary_list.append("1" if bit == "0" else "0" )

can also be written:

    binary_list = ["1" if bit == "0" else "0" for bit in self.binary_value]

We get

def flip_bits(self):
    binary = "".join(["1" if bit == "0" else "0" for bit in self.binary_value])
    return Binary(self.bits).from_binary(binary)

Improving def positive_binary_convert(self, integer)

We can remove the finished variable: we could just break out of the loop for the time being.

We can remove the useless conversions int(integer).

We could once more use divmod.

We could use in-place operator.

At this stage, we have:

def positive_binary_convert(self, integer):
    before = ""
    while True:
        divided, remainder = divmod(integer, 2)
        before += str(remainder)
        integer = divided
        if divided == 0:
            break

Also, the divided variable is not useful. We could use integer directly.

Improving def convert_int_to_binary(self, number)

You could use directly:

    neg = number < 0

Also, you don't even need that variable at all:

def convert_int_to_binary(self, number):
    binary = self.positive_binary_convert(abs(number))
    if number < 0:
        binary = binary.change_state()
    return binary.binary_value

General comment

There is something I find particularly fishy in the from_binary method. From the name, it looks like it should be a class method (used as an alternative constuctor) but it is actually a normal method.

I've tried to re-write the code with a simple constructor and 2 alternative constructors (from int and from binary) and got the results below but I am still not fully convinced:

# https://codereview.stackexchange.com/questions/214533/python-binary-calculator

class Binary:
    def __init__(self, bits):
        self.bits = bits 
        self.binary_value = None

    @classmethod
    def from_int(cls, bits, integer: int):
        self = cls(bits)
        self.binary_value = self.convert_int_to_binary(integer)
        return self

    @classmethod
    def from_binary(cls, bits, binary: str):
        self = cls(bits)
        self.binary_value = self.allocate_bits(binary[::-1])[::-1]
        return self

    def __str__(self):
        return self.binary_value

    def __len__(self):
        return len(self.binary_value)

    def __add__(self, other):
        final_added = ""
        carry = False

        for i, j in zip(reversed(self.binary_value), reversed(other.binary_value)):
            carry, bit = divmod(int(i) + int(j) + carry, 2)
            final_added += str(bit)
        # TODO: Handle inputs of different length or carry overflow ?
        return Binary.from_binary(self.bits, final_added[::-1])

    def __sub__(self, other):
        return self + (- other)

    def __mul__(self, other):
        return sum(
                (Binary.from_binary(self.bits, str(int(self.binary_value) * int(val) * int(10 ** i)))
                    for i, val in enumerate(reversed(other.binary_value))),
                Binary.from_int(self.bits, 0))

    def __ge__(self, other):
        return self.denary() >= other.denary()  # I know this isn't ideal but will come back around to it.

    def __le__(self, other):
        return self.denary() <= other.denary()

    def __truediv__(self, divisor):
        dividend = self.binary_value
        cur_div = ""
        answer = ""
        for i in range(0, len(dividend)):
            cur_div = cur_div + dividend[i]
            if divisor <= Binary.from_binary(self.bits, cur_div):
                answer = answer + "1"
                take_away = divisor * Binary.from_binary(self.bits, answer)
                cur_div = (Binary.from_binary(self.bits, cur_div) - take_away).binary_value

            else:
                answer = answer + "0"

        print(answer)
        answer = Binary.from_binary(self.bits, answer)
        print(answer.denary())
        return self

    def __neg__(self):
        return self.change_state()

    def denary(self):
        first_bit = self.binary_value[0]
        remaining = self.binary_value[1:]
        sign = 1
        if first_bit == "1":
            sign = -1
            remaining = self.change_state().binary_value[1:]
        return sign * sum(((2 ** i) * int(val)) for i, val in enumerate(reversed(remaining)))

    def convert_int_to_binary(self, number):
        binary = self.positive_binary_convert(abs(number))
        if number < 0:
            binary = binary.change_state()
        return binary.binary_value

    def positive_binary_convert(self, integer):
        before = ""
        while True:
            integer, remainder = divmod(integer, 2)
            before += str(remainder)
            if integer == 0:
                break
        binary = self.allocate_bits(before)[::-1]  # Flips the string
        return Binary.from_binary(self.bits, binary)

    def change_state(self, binary=None):  # Change from negative to positive
        if binary is not None:
            flipped = Binary.from_binary(self.bits, binary).flip_bits()
        else:
            flipped = self.flip_bits()

        return flipped + Binary.from_int(self.bits, 1)

    def allocate_bits(self, value):
        length = len(value)
        if length < self.bits:
            rem = self.bits - length
        else:
            rem = length % self.bits
        return value + "0" * rem

    def flip_bits(self):
        binary = "".join(["1" if bit == "0" else "0" for bit in self.binary_value])
        return Binary.from_binary(self.bits, binary)


def test_equal(binary_val, expected_res):
    res = binary_val.denary()
    if res == expected_res:
        print("Passed")
    else:
        print("Failed", res, expected_res)

def test(bits = 64):
    test_equal(Binary.from_int(bits, 5) + Binary.from_int(bits, 10), 15)
    test_equal(Binary.from_int(bits, 125) + Binary.from_int(bits, 125), 250)
    test_equal(Binary.from_int(bits, 5) - Binary.from_int(bits, 10), -5)
    test_equal(Binary.from_int(bits, 255) - Binary.from_int(bits, 120), 135) 
    test_equal(Binary.from_int(bits, 4) * Binary.from_int(bits, 5), 20)
    test_equal(Binary.from_int(bits, 125) * Binary.from_int(bits, 1000), 125000)

test()