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())
-
1\$\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– Roland Illig2019年06月08日 08:46:56 +00:00Commented Jun 8, 2019 at 8:46
-
\$\begingroup\$ Thank you. I will make sure to follow these guidelines \$\endgroup\$brandontaz2k2– brandontaz2k22019年06月08日 10:35:18 +00:00Commented Jun 8, 2019 at 10:35
1 Answer 1
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 ValueError
in 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
- Use
snake_case
for variable and function names andCamelCase
/PascalCase
for class names. - 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
- converting the number
- printing the converted number
- 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.
-
\$\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– brandontaz2k22019年06月08日 10:25:25 +00:00Commented Jun 8, 2019 at 10:25
-
1\$\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– AlexV2019年06月08日 14:15:54 +00:00Commented Jun 8, 2019 at 14:15
Explore related questions
See similar questions with these tags.