3
\$\begingroup\$

Is this the most efficient way to write this piece of code? Is there a better way of handling errors inside of a while/for loop, instead of calling back to the original function? I have tried using break or continue, but it breaks the functionality of my program. Any help would be awesome, thanks.

## calculate binary to decimal ##


def binaryToDecimal(binary):
    ### reverse the number
    decimal_num = 0
    reverse_binary = binary[::-1]
    for x in range(7,-1,-1):
        if len(binary) <= x:
            continue
        if int(reverse_binary[x]) == 1:
            decimal_num += 2**x
    print(f"Your binary number converts to {str(decimal_num)}. ")
    binaryToDecimal(checkUI())


def getUserInput():
    return input("Please enter a binary number that you "
                 "want to convert into a decimal.\n Type 'quit' to quit program.\n")


def checkUI():
    userInput = getUserInput()
    if userInput == "quit" or userInput == "q":
        quit()
    for character in userInput:
        try:
            if len(userInput) > 8:
                print("Binary numbers are a maximum of 8 digits. Please try again")
                binaryToDecimal(checkUI())
            val = int(character)
            if val != 1 or 0:
                print("Invalid binary number. Please enter a 1 or 0 (binary numbers: 11110000)")
                binaryToDecimal(checkUI())
        except ValueError:
            print("You did not enter a binary number. (11110000) Please try again.")
            binaryToDecimal(checkUI())

    return str(userInput)


binaryToDecimal(checkUI())
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review. Before posting your next question, please read how to ask questions here. The introductory text in a question should describe what the code is supposed to do, and in this question you didn't mention this at all. The title of the "question" should describe what the code does, and not that you need help. \$\endgroup\$ – Roland Illig Jun 8 at 8:46
  • \$\begingroup\$ Thank you. I will make sure to follow these guidelines \$\endgroup\$ – brandontaz2k2 Jun 8 at 10:35
3
\$\begingroup\$

TL;DR:

It's unlikely to beat Python's own implementation of this which is basically as simple as

decimal = int(binary, base=2)

in terms of convenience, clearity, and performance. This will throw a ValueErrorin case the number cannot be converted to a binary, e.g. if the user enters something like 101010103.

Welcome to the world of Python, where "batteries included" is a real thing!

If you, for whatever reason, do not want to use the most convenient Python has to offer, it is still possible to write a Pythonic alternative:

decimal = sum(int(digit)* 2**index for index, digit in enumerate(user_input[::-1]))

This little piece of code is called generator expression (relevant PEP, good Stack Overflow post) and is basically the equivalent of:

decimal = 0
for exponent, digit in enumerate(user_input[::-1]):
    decimal += digit * 2**exponent

which is close(r) to what you did in your original implementation. Apart from that you don't have to care about the actual length of the input.

Detailed review

Of course there is more to say about your code than this, especially since you're asking about other aspects as well. So let's look at your code.

Style

Python has an official Style Guide called PEP8 (batteries included, you remember?). The main takeaways that are follow by the better part of Python programmers are

  1. Use snake_case for variable and function names and CamelCase/PascalCase for class names.
  2. Write function documentation (often just docstrings) """Enclosed in triple quotes""".

I will demonstrate how to apply these principles to your code in a moment. There are other useful recommendations which help you to keep the appearance of your code clean and readable, so it's definitely worth a read.

Naming

Apart from the stylistic point in the names, there is always the name itself. It's good that you decided to use functions and most of their names are totally fine. The odd one out is clearly checkUI. If you were just left with this name, I highly doubt that you would be able to tell what it does. Since it basically handles most of the interaction with the user, as well as validating and converting the numbers, the obvious (though maybe not ideal) name would be simply main().

The script itself

After we have gone through some high-level aspects of the code, let's dive into the details. I will go through the code function by function.

binaryToDecimal - soon to be binary_to_decimal

As I already said above, the core functionality of the function can basically be replaced by a single line of Python code. And that's likely all the function should do. Apart from the main function, all of them should basically have a single responsibility. At the moment there are three

  1. converting the number
  2. printing the converted number
  3. starting a new round of user input

The 2nd one is easy to fix: simple move the call to print from binary_to_decimal to main. The 3rd one is really, really (and I mean really really) bad design. The conversion function should absolutely not be tasked with starting a new round of user input. To make it worse, this will also lead to recursive calls of your functions, that will eventually end with a RuntimeError when your program exceeds the maximum recursion depth allowed by the Python interpreter. So drop that and never do it again!

getUserInput - soon to be get_user_input

The function is basically fine and a prime example of single responsibility. Maybe it's functionality is even a bit to minimalistic, so you might not actually need it.

checkUI - soon to be main

This is where things get serious. The first and foremost recommendation for this piece of code is similar to the first function we looked at: Get rid of the recursion!. Recursion as such is not a bad thing and valid programming technique in their own right, but not for this purpose!

The idiomatic way to ask for user input repeatedly would be something like

def main():
    """Repeatedly ask for user input and convert its decimal equivalent"""
    while True:
        # read and normalize the user input
        # Note: this might also (better) be done in get_user_input
        user_input = get_user_input().strip().lower()
        if user_input.startswith("q")
            break

        # further validation and conversion
        ...

if __name__ == "__main__":
    main()

The check to stop the program was slightly modified to be a little bit more robust. The way I chose to implement it will also make sure that things like Q, Quit, qsomethingelse, and the like end the program. If that's not what you want, simply replace it with the check you had originally. I also added if __name__ == "__main__": which adds as a kind of safeguard to make sure the "interactive" of your code is only triggered if you use it as script and not try to import it.

Now for the validation and conversion part. With all the other changes in place it will become as simple as

def main():
    """Repeatedly ask for user input and convert its decimal equivalent"""
    while True:
        # read and normalize the user input
        # Note: this might also (better) be done in get_user_input
        user_input = get_user_input().strip().lower()
        if user_input.startswith("q"):
            break

        if len(user_input) > 8:
            print("Binary numbers are a maximum of 8 digits. Please try again")
            continue

        try:
            decimal = binary_to_decimal(user_input)
            print(f"Your binary number converts to {decimal}. ")
        except ValueError:
            print("You did not enter a binary number. (11110000) Please try again.")

Since you chose to limit binary numbers to a maximum length of 8bit (why?), you should likely also include this in the prompt.

\$\endgroup\$
  • \$\begingroup\$ Hi Alex, thanks for your detailed response! I am a noobie to Python coding, this was just a fun side learning project for me during my Computer Networking class. I know I could have just used what is already built into Python, but I wanted to learn a little bit about Python and how to code efficiently and effective. I didn't even know about docstrings, they certainly look cleaner than just a normal comment. I limited the binary numbers to 8bits because I was working with IPv4 subnets in my Computer Networking class. Now that I see the cleaned up code everything makes so much more sense. Thanks \$\endgroup\$ – brandontaz2k2 Jun 8 at 10:25
  • \$\begingroup\$ @brandontaz2k2: I added an alternative, more "Pythonic" implementation of your original code, to show some more of the cool things Python has to offer. \$\endgroup\$ – AlexV Jun 8 at 14:15

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.