5
\$\begingroup\$

Here I am doing some exercises for my studies with Python. This is a binary to decimal number converter (currently restricted to that).

These are the very first steps, so there is no error handling. You can even input non-binary numbers and get a result. However it works for converting binary numbers, which is its task.

I'm not sure about all shenanigans going around with the main function and the argument parsing.

import argparse
import sys

###############################################################################

def main(binary_number):
    binary_number = list(binary_number)
    decimal_number = 0

    for digit in range(0, len(binary_number)):
        if binary_number[digit] != '0':
            decimal_number += power_to_the_people(2, len(binary_number)-digit-1)

    print(decimal_number)

###############################################################################

def power_to_the_people(base, exponent):
    # Return 1 if exponent is 0
    if exponent == 0:
        return 1

    power = base
    for j in range(1, exponent):
        power *= base

    return power # to the people

###############################################################################

if __name__ == '__main__':

    parser = argparse.ArgumentParser(description='Converts a binary to a decimal number.')

    parser.add_argument('binary_number', action="store")

    args = parser.parse_args()

    main(args.binary_number)

(Update: Added parentheses to print so it actually works under 3.4)

\$\endgroup\$
4
\$\begingroup\$

A few additional points for your learning experience.

You're not using sys, so the import line is unnecessary.

In range(...), the default start is 0, so instead of range(0, x) you can write simply range(x).


Don't use pointless comments like this:

# Return 1 if exponent is 0
if exponent == 0:
    return 1

Instead if this:

power = base
for j in range(1, exponent):
    power *= base

It would be better this way:

power = 1
for j in range(exponent):
    power *= base

Because in this version base is referenced only once, and range is simplified by dropping 1 to use the default 0.


action="store" is unnecessary here:

parser.add_argument('binary_number', action="store")

main is not a good name for that function does. It would be better to rename it to binary_to_decimal, for example.

And while at it, it would be better to move the content from the if __name__ ... inside a new main method, like this:

def main():

    parser = argparse.ArgumentParser(description='Converts a binary to a decimal number.')
    parser.add_argument('binary_number')
    args = parser.parse_args()
    binary_to_decimal(args.binary_number)

if __name__ == '__main__':
    main()

Many people miss this point, actually. The reason to do it this way is that code in the if __name__ ... block is in the global namespace. So variables with the same name in the rest of the code will shadow those variables, which can lead to nasty bugs. By moving that code to a method, this cannot happen.

PEP8

PEP8 is the official style guide of Python, and you have a few violations:

  • Put 2 blank lines before each method definition and you only put 1.

  • Put 2 spaces in front of # inline comments

  • Lines should not be longer than 79 characters

\$\endgroup\$
  • \$\begingroup\$ import sys was a remnant from trying to figure out how to do Python scripts with command line arguments. Thank you for the helpful tips, especially regarding the main() function. \$\endgroup\$ – kleinfreund Nov 13 '14 at 19:26
3
\$\begingroup\$

That is nice-looking code, but it's also quite long for the problem that you are solving. The simple solution would be print(int(binary_number, 2)).

Assuming that you want to solve the problem the hard way as an exercise, it's a good idea to separate your calculation routines from your input/output routines. You need a binary_to_int(binary_string) function. It can be implemented using a much simpler algorithm.

def binary_to_int(binary_string):
    n = 0
    for char in binary_string:
        n *= 2
        if char == '1':
            n += 1
    return n
\$\endgroup\$
  • \$\begingroup\$ Note this is MSB-first ordering. binary_to_int('0001') returns 1 and binary_to_int('1000') returns 8. Perfectly normal, just thought to mention it. \$\endgroup\$ – user27318 Nov 12 '14 at 10:15

protected by Community Jan 29 '16 at 12:41

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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