5
\$\begingroup\$

I want you to pick my code apart and give me some feedback on how I could make it better or more simple. In my opinion this is the best code I have written, being that I'm new to programming. The program is a basic "converter" that has the user input his/her age and it tells how many days, hours, and minutes the user has been alive. Let me know what you think!

import time
import sys

def name():
    name = input("What is your name? ")
    while name.isdigit():
        print("Make sure to enter an actual name.")
        name = input("What is your name? ")
    return name

def user_input():
    while True:
        age = input("What is your age? ")
        try:
            return int(age)
            break
        except ValueError:
            try:
                return float(age)
                break
            except ValueError:
                print("Make sure to enter a real age.")

def again():
    while True:
        again = input("Play again? Yes/No ").lower()
        if again == "no":
            sys.exit(0)
        elif again == "yes":
            break
        else:
            print("That was not an option.")

while True:
    the_name = name()
    age = user_input()
    Days = age * 365
    Hours = Days * 24
    Minutes = Hours * 60
    print("Ok {}, I have your results.".format(the_name))
    time.sleep(2)
    print("If you are {} years of age....".format(age))
    time.sleep(2)
    print("You are {} days old.".format(Days))
    time.sleep(2)
    print("You are {} hours old.".format(Hours))
    time.sleep(2)
    print("You are {} minutes old!".format(Minutes))

    again()

NOTE: If you make a change, please explain to me why and how it works in the most simple way you can, so I understand it better.

\$\endgroup\$
  • \$\begingroup\$ Why not ask for a birth date (and time?) so you can calculate the real values? \$\endgroup\$ – Graeme Stuart Sep 15 '13 at 23:03
  • \$\begingroup\$ Can you create an answer and show me how you would write the code to do that? @GraemeStuart \$\endgroup\$ – TheSuds13 Sep 15 '13 at 23:44
  • \$\begingroup\$ OK I have added an answer @TheSuds13. \$\endgroup\$ – Graeme Stuart Sep 18 '13 at 21:18
6
\$\begingroup\$

This:

def name():
    name = input("What is your name? ")
    while name.isdigit():
        print("Make sure to enter an actual name.")
        name = input("What is your name? ")
    return name

means that "1" is not a real name, but "a" and "17xy" are. I think it might be better to use some kind of a regular expression, or - better - just accept whatever nonempty string the user chooses to give you as his/her name.

In your user_input(), you try to convert the input to int, and - if that fails - to float. Why not straight to float? I don't see how is 17 better than 17.0.

Also, in the same function, your break statements will never execute, as the return statement terminates the function.

I think it'd be a much better program if the input was date of birth. Do you know your current age, as anything more precise than "between x and x+1 years old"?

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

Why not ask for a birth date (and time?) so you can calculate the real values. You will need to import datetime.

import time
import sys
from datetime import datetime

This function is OK, though I am not sure whether your isdigit test will catch many mistakes

def get_name():
    name = raw_input("What is your name? ")
    while name.isdigit():
        print("Make sure to enter an actual name.")
        name = raw_input("What is your name? ")
    return name

Your function name, user_input() is too vague. I have changed it to get_date_of_birth() to make it more obvious what's going on Your original may be better named as get_age() for example I have also simplified the logic, your return statement is enough, no need to break I have also converted the input to a date using datetime.strptime()

def get_date_of_birth():
    date_string = raw_input("What's your date of birth? ")
    while True:
        try:
            return datetime.strptime(date_string, '%d/%m/%Y')
        except ValueError:
            date_string = raw_input("What's your date of birth? (format: dd/mm/yyyy) ")

Your again() function is great. I have simplified by returning a boolean value from the function which allows me to use it differently in the main logic below. I have also added single character options using 'in'

def again():
    while True:
        again = raw_input("Play again? (Yes/No) ").lower()
        if again in ["no", "n"]:
            return False
        elif again in ["yes", "y"]:
            return True
        else:
            print("That was not an option.")

I have moved the main game flow into a function. Here I have changed it to use a datetime as the user's date of birth.

def play():
    name = get_name()
    dob = get_date_of_birth()
    age = datetime.today() - dob
    print("Ok {}, I have your results.".format(name))
    time.sleep(1)
    print("You are {} years and {} days old....".format(age.days // 365, age.days % 365))
    time.sleep(1)
    print("You are {} days old.".format(age.days))
    time.sleep(1)
    print("You are {} hours old.".format(int(age.total_seconds() // (60*60))))
    time.sleep(1)
    print("You are {} minutes old!".format(int(age.total_seconds() // 60)))

The main logic is reduced to a simple, obvious loop. Code that executes should be protected behind if name == "main" so it doesn't run on import.

if __name__ == "__main__":
    play()
    while again():
        play()

I'm using raw_input() rather than input() as my code is python 2.7. I guess you are using python 3.

| improve this answer | |
\$\endgroup\$

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.