3
\$\begingroup\$

I am a beginner in Python and I have written a program to convert binary numbers to decimal. It works, but are there any ways to improve the code and make it better?

def binary_to_decimal_converter(binary_value):
    print(f"Binary: {binary_value}")
    decimal_value = 0
    # Treating binary input as a string
    # power is power of number 2
    # power must be less than string length because computer starts counting from 0th index
    power = len(str(binary_value)) - 1
    loop_var = 0
    while loop_var < len(str(binary_value)):
        if str(binary_value)[loop_var] == str(1):
            decimal_value += 2 ** power
        power -= 1
        loop_var += 1
    return print(f"Decimal: {decimal_value}")


# Calling the function
binary_to_decimal_converter(1111101100)

Output:

Binary: 1111101100
Decimal: 1004

```
\$\endgroup\$
6
  • 1
    \$\begingroup\$ Does this function really do what you meant to do? The input isn't really in binary, it's sort of "decimal coded binary" (one decimal digit encodes one binary digit). That's very strange. \$\endgroup\$ – harold Aug 16 '20 at 12:39
  • \$\begingroup\$ yeah the input is string.... the fucntion does the job.... but im not sure about the logic... and that's what im trying to figure out. Im a total newbee \$\endgroup\$ – Ak119 Aug 16 '20 at 12:46
  • 1
    \$\begingroup\$ Well that's the strange thing, the input isn't a string, but it could have been and then you wouldn't need str(binary_value) \$\endgroup\$ – harold Aug 16 '20 at 12:57
  • \$\begingroup\$ Oh... Okay.. Understood. Thanks a lot :) \$\endgroup\$ – Ak119 Aug 16 '20 at 13:08
  • \$\begingroup\$ I think this is off-topic, as your code really doesn't do the job. You don't take binary (you take an int) and you don't convert to decimal (you produce an int). The only place converting to decimal is when you print an int, and that's not your code doing the job. \$\endgroup\$ – Kelly Bundy Aug 16 '20 at 13:15
2
\$\begingroup\$

Do conversions once

As written, with the example input, str(binary_value) will be called 22 times, with the same input value, and returning the same value each time. This is very inefficient; it is wasted work. It would be better to call it once, and store the result, and use this stored value:

def binary_to_decimal_converter(binary_value):
    binary_value = str(binary_value)
    # ... rest of function, without any more str(binary_value) calls

But ...

Representing Binary Numbers

As pointed out by harold in the comments, the literal 1111101100 is an integer value slightly larger than one billion in Python. You should use a string to represent a binary number to be converted.

# Calling the function
binary_to_decimal_converter("1111101100")

Since we're passing a string to the function, all of the calls to str(binary_value) are now completely unnecessary.

String Constants

        if str(binary_value)[loop_var] == str(1):

Again, using the example value, the expression str(1) will be evaluated ten times; once for each digit of the input. This will convert the numerical value one into the string "1", which will be discarded immediately after use, only to be recreated on the next iteration. Why not just use the literal "1"? Not only is it more efficient; it is 3 characters shorter.

Loop like a Native

See "Loop like a Native" by Ned Batchelder on YouTube for more details, motivation, etc.

Part 1

Our modified code now looks like this:

    loop_var = 0
    while loop_var < len(binary_value):
        # ...
        loop_var += 1

len(binary_value) is computed once for each iteration through the loop; 11 times, including the last pass where the condition evaluates to False and the loop terminates. Since the string doesn't change inside the loop, the length won't change either, so this value is a constant.

    loop_var = 0
    limit = len(binary_value)
    while loop_var < limit:
        # ...
        loop_var += 1

Now that we've recognized that limit is fixed, we can see this is actually a for loop, that starts at zero, counts up by 1, and stops when it reaches limit. Instead of manually adjusting the loop_var by 1, and manually doing the comparison, we should simply use the for loop, over the required range, which is much more efficient:

    for loop_var in range(len(binary_value)):
        # ...

Part 2

Our loop now looks like:

    for loop_var in range(len(binary_value)):
        if binary_value[loop_var] == "1":
            # ...
        # ...

Indexing is expensive in Python. Anytime you see a for loop, using a range(len(thing)), and the loop variable is only used as a index into the thing which the range is looping over, thing[loop_var], then we want to let Python do the indexing for us in the for statement:

    for digit in binary_value:
        if digit == "1":
            # ...
        # ...

Return Value

    return print(f"Decimal: {decimal_value}")

Is this function returning a value, or is it printing the result? Currently, it is taking the value which is returned by the print() function, and returning that. Ie)

    temporary = print(f"Decimal: {decimal_value}")
    return temporary

But print() returns None, so this is effectively:

    print(f"Decimal: {decimal_value}")
    return None

And if the function ends without a return statement, it automatically returns None anyway. So the return can simply be omitted.

Note: It is better to return results from a function that does a calculation, without printing anything inside the function, and have the caller do the printing. This will create more flexible code in the future. Left to student.

Improved code

def binary_to_decimal_converter(binary_value):
    print(f"Binary: {binary_value}")
    decimal_value = 0

    # power is power of number 2
    # power must be less than string length because computer starts counting from 0th index
    power = len(binary_value) - 1

    for digit in binary_value:
        if digit == "1":
            decimal_value += 2 ** power
        power -= 1

    print(f"Decimal: {decimal_value}")


# Calling the function
binary_to_decimal_converter("1111101100")
\$\endgroup\$
4
  • 1
    \$\begingroup\$ You should add a link to NetBat's talk if you're going to use his title: youtu.be/EnSu9hHGq5o \$\endgroup\$ – aghast Aug 17 '20 at 17:18
  • \$\begingroup\$ Meh, this still only does half the job. It now takes a binary number, but still only produces an int, not a decimal number. The whole work of converting the int to decimal is done by the f-string. You might as well cheat for the first half of the job as well and use int(binary_value, 2). Btw, your digits are binary digits, a.k.a. bits, so I'd use the variable name bit. \$\endgroup\$ – superb rain Aug 17 '20 at 19:25
  • \$\begingroup\$ @superbrain It does what the original program appears to be intended to do, with didactic improvements aimed at the apparent level of the OP. It is tagged reinventing-the-wheel (admittedly, by me) because the OP is clearly trying to implement the binary-to-integer conversion themselves. If you feel this answer is incomplete, and only doing half the job, please provide your own complete answer. \$\endgroup\$ – AJNeufeld Aug 17 '20 at 19:45
  • \$\begingroup\$ @AJNeufeld Thanks a lot for your time and input. It helps a lot. \$\endgroup\$ – Ak119 Aug 21 '20 at 4:54
1
\$\begingroup\$
  • Personally, I find the function name binary_to_decimal_converter more explicit than necessary, I'd call it binary_to_decimal.

  • Your input type is wrong. A binary number is a string. Your input is a number (technically an int). Numbers don't have digits/bits/whatever and aren't binary/decimal/whatever. Number representations have digits/bits/whatever and are binary/decimal/whatever. At most you could argue that int is internally a binary representation, but that wouldn't really be true (I think it's base 231 or so) and also none of our business. For all we know, it's just a number, and we can do number things with it. Like multiplying it with other numbers. But you can't ask it for a digit. Because it doesn't have any. Try 3141[2], you'll get an error. Unlike '3141'[2], which gives you '4'. So since you're talking about binary and decimal, your input and output should be strings. (End of numbers-vs-number-representations rant :-)

  • You compute the number from the "binary" with your own code, but then you make the f-string do the whole second half of the job, conversion from the number to decimal. Rather seems like cheating.

  • It's inefficient and more complicated than necessary to use powers like you do. Instead, you could just double the current value before adding the next bit.

Code doing all that, converting the given binary to a number (int) and then to decimal:

def binary_to_decimal(binary):
    number = 0
    for bit in binary:
        number *= 2
        if bit == '1':
            number += 1
    decimal = ''
    while number:
        digit = '0123456789'[number % 10]
        decimal = digit + decimal
        number //= 10
    return decimal or '0'

Test:

>>> binary_to_decimal('1111101100')
'1004'
\$\endgroup\$
1
  • \$\begingroup\$ Thanks a lot for the input... makes a lot of sense now. \$\endgroup\$ – Ak119 Aug 21 '20 at 13:11

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.