2
\$\begingroup\$

I started learning Python 2 about a week ago, using Codecademy and Stack Overflow answers to help me learn. I wrote a code to calculate the user's free time based on user sleep and work time. It also lets the user decide whether to see percentage of free time. It's not fancy; I never intend to actually use it. It works the way I want it to.

I'm looking for feedback on how I wrote this code. What should I have done differently?

print "The program is operating."

def sleep(time):
  sleep_hours = float(raw_input("How many hours do you sleep a night?: "))
  return time - sleep_hours

def work(time):
 work_hours = float(raw_input("How many hours do you work each weekday?: "))
 time -= work_hours
 return time

def see_percent():
  reply = raw_input("Would you like to know the percent of your waking time is free time? (Y/N): ").lower()
  if reply == "y":
    print percent_round + "%."
  elif reply == "n":
    print "Okay"
  else:
    print "Y/N means enter a 'Y' for yes or a 'N' for no."


time = 24.
time_minus_sleep = sleep(time)
time_minus_work = work(time_minus_sleep)
waking_time = time_minus_sleep
percent_free = str(time_minus_work / waking_time * 100)
percent_round = percent_free[:5]

print""
print "You have %.2f hours of freetime." % time_minus_work
waking_time = time_minus_sleep
print "Of the %.0f hours in a day, you are awake for %.2f of them." % (time, waking_time)
\$\endgroup\$
2
\$\begingroup\$

There is no need for sleep and work to take the time as an argument. Just return what the user enters and calculate in the main function. The main part of the code should be guarded by a if __name__ == "__main__": guard, which ensures that that code is only executed when running the script but not when importing from it.

Python 2 will soon be no longer supported. If you are starting to learn Python now, learn Python 3 straight away. In the code below the main differences are that raw_input is now input (and the Python 2 input no longer exists), print is no longer a statement, but a function (so surround it with (), instead of ), division is now float division by default (use // for integer division) and finally string formatting has become a lot easier in Python 3.6 with f-strings.

def ask_time(message):
    while True:
        try:
            hours = float(input(message))
            if 0 <= hours <= 24:
                return hours
        except ValueError:
            pass
        print("Please enter a number between 0 and 24")

def see_percent():
    reply = input("Would you like to know the percent of your waking time is free time? (Y/N): ").lower()
    return reply == "y"


if __name__ == "__main__":
    total_time = 24.  # hours
    sleep_time = ask_time("How many hours do you sleep a night?: ")
    work_time = ask_time("How many hours do you work each weekday?: ")

    waking_time = total_time - sleep_time
    free_time = waking_time - work_time
    print(f"You have {free_time:.2f} hours of free time.")
    print(f"Of the 24 hours in a day, you are awake for {waking_time:.2f} of them.")

    if see_percent():
        percent_free = free_time / total_time
        print(f"{percent_free:.2%}.")

I also made a more general ask_time function which ensures that the user enters a valid number between 0 and 24 and keeps on asking if they didn't and used four spaces throughout, as recommended by Python's official style-guide, PEP8.

\$\endgroup\$
0
\$\begingroup\$

My implementation of the program: (Comments, explanation of changes below).

print "The program is operating."

def sleep(time):
  sleep_hours = float(raw_input("How many hours do you sleep a night?: "))
  return time - sleep_hours

def work(time):
  work_hours = float(raw_input("How many hours do you work each weekday?: "))
  return time - work_hours

def see_percent():
  reply = raw_input("Would you like to know the percent of your waking time is free time? (Y/N): ").lower()
  if reply == "y":
    print percent_round + "%."
  elif reply == "n":
    print "Okay"
  else:
    print "Y/N means enter a 'Y' for yes or a 'N' for no."


time = 24.
time = sleep(time)
waking_time = time
time = work(time)
percent_free = str(time / waking_time * 100)
percent_round = percent_free[:5]

print "\nYou have %.2f hours of freetime." % time
print "Of the 24 hours in a day, you are awake for %.2f of them." % waking_time

see_percent()

Additions

  1. I added a call for the "see_percent()" function at the bottom of the code.

Subtractions (Top to bottom)

  1. I removed the time assignment in the "work()" function, as I could perform the operation in the return statement, saving a line.
  2. I removed the "time_minus_sleep" variable, as I was able to achieve the output of the original program just by changing the "time" variable.
  3. I removed the first print statement, and added an escape sequence "\n" to create a newline instead of using the separate print statement.
  4. I removed the second redundant waking_time assignment, as it was no longer needed.
\$\endgroup\$
0
\$\begingroup\$

I think calling those functions work and sleep is confusing. sleep doesn't actually "sleep" anything, it's asking for the number of hours slept. I would change the names to something like ask_sleep_time to make it clearer what the purpose is.

sleep will also cause conflicts if you happen to do from time import sleep in the future.


And I completely agree with @Graipher's ask_time function. That is what immediately jumped into my head when I saw your sleep and work functions. They have two big problems.

  • They're both nearly the same code (ask for an input, subtract from time).

  • They're doing too much.

For the second point, why are the functions subtracting from time? This means that you have them doing two jobs.

  • Getting user input.

  • Calculating and returning the time remaining.

In this particular case this isn't a huge deal, but it should be kept in mind. When you start writing larger, more complicated functions, you need to make sure you don't have single functions doing multiple, unrelated tasks when possible. Such functions make testing harder and code more difficult to understand.


I also disagree with the name time. 1. time is the name of a standard python module. You don't want to run into conflicts with it, or allow for confusion. I think it would make more sense to have a hours_in_day = 24 variable, then do something like:

sleep_hours = ask_time("How many hours do you sleep a night?: ")
work_hours = ask_time("How many hours do you work each weekday?: ")

free_hours = hours_in_day - sleep_hours - work_hours

Similar to what @Graipher has.

\$\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.