I created a Python program for one of CS50's problem sets that converts time input by the user into a float—like changing '7:30' into '7.5'. I want to improve the readability of my function. Here's what it looks like:
```python
def convert(time):
# Converts time to a float in a.m.
if time.lower().endswith("a.m."):
hours, minutes = time.lower().replace("a.m.", "").split(":")
minutes = int(minutes)
minutes = int(minutes * (5 / 3))
hours = int(hours)
time = str(hours) + "." + str(minutes)
time = float(time)
return time
# Converts time to a float in p.m.
elif time.lower().endswith("p.m."):
hours, minutes = time.lower().replace("p.m.", "").split(":")
minutes = int(minutes)
minutes = int(minutes * (5 / 3))
hours = int(hours)
time = str(hours) + "." + str(minutes)
time = float(time) + 12
return time
# Converts time to a float in 24-hour format
else:
hours, minutes = time.split(":")
minutes = int(minutes)
minutes = int(minutes * (5 / 3))
hours = int(hours)
time = str(hours) + "." + str(minutes)
time = float(time)
return time
```
I'd love to hear your suggestions on how to make it clearer and more efficient!
4 Answers
I noticed that you have this repeated block in all three cases:
```python
minutes = int(minutes)
minutes = int(minutes * (5 / 3))
hours = int(hours)
time = str(hours) + "." + str(minutes)
```
You could extract this into a helper function that takes `hours` and `minutes` and returns a formatted float directly. Also, there’s no need to cast hours to int and back to str; you can just do the conversion to float in one step at the end.
You've got three blocks of code doing nearly the same thing. Consider moving the common logic outside the if-else structure and only keeping the unique parts for each time format. It'll tidy things up a lot!
You could simplify your input parsing by using regular expressions. This would help you capture different time formats without the need for if-else checks. For example:
```python
import re
pattern = r's?[apAP].?[mM].?'
new_time = re.sub(pattern, '', time)
```
This would catch all variations like '1:00 a.m.', '1:00 A.M.', etc.
Your function has a lot of repeating code! Try to extract that repeating logic into a separate helper function. Additionally, your `* (5 / 3)` can be confusing; it’s clearer to use numbers that convey their purpose like `* (100 / 60)`, or better yet, define a constant like `MINUTES_TO_DECIMAL = 100 / 60` to enhance readability. Avoid using magic numbers as much as possible.
Thanks for the tips! I'll definitely try simplifying the repetitive parts.
Great suggestion! That should help a lot.