[Tutor] Help! (solution)

Alan Gauld alan.gauld at btinternet.com
Sat Mar 5 10:25:31 CET 2011


"michael scott" <jigenbakuda at yahoo.com> wrote
>...if something about my code could be better, please tell me,

OK, Here are some stylistic things...

> def production_time():
>    creation_time = 127
>    time_till_rest = 18161

I'd move the constants out of the function to global level.
And it is convention to make constants all CAPS:

CREATION_TIME = 127

Also rather than arbitrary values show how these are arrived at:

NUMBER_TILL_REST = 143
TIME_TILL_REST = CREATION_TIME * NUMBER_TILL_REST


>    items = raw_input("How many items will be produced?\n> ")
>    item_time = int(items) * creation_time
>    rest_times = item_time/time_till_rest
>    print rest_times
>    if rest_times > 0:
>        total = item_time + (313 * rest_times) #313 is 5 min and 13 
> secs in
> second form

Rather than the "magic number" 313 I'd make it another constant.:

REST_TIME = (5 * 60) + 13   # in seconds

>    else: total = item_time
>    time = sec_to_standard(total)
>    print "It would take %d days %d hours %d minutes and %d seconds 
> to produce
> that many items" %(time[0], time[1], time[2], time[3])
>
>
> def sec_to_standard(seconds):
>    day = 86400 #secs
>    hour = 3600 #secs
>    mins = 60#seconds
>    creation_time = 127 #secs
>    time_till_rest = 18161 #secs

More constants and some are duplicates.
Take them outside and you only have to change them in one place
so mainmtenance becomes easier. And the times get expressed as 
calculations:

MIN=60
HOUR = MINS * 60
DAY = HOUR * 24
REST_TIME=(5* MIN)+13

>    days = 0
>    hours = 0
>    minutes = 0
>    secs = 0

Some prefer the single line style of

days = hours = minutes = secs = 0

>    if seconds > day:
>        while seconds > day:
>            print "doing days"
>            seconds = seconds - day
>            days += 1

This is a complicated way of doing modulo division!

days,seconds = divmod(seconds,DAY)

>    if seconds > hour:
>        while seconds > hour:
>            print "doing hours"
>            seconds = seconds - hour
>            hours += 1

hours,seconds = divmod(seconds,HOUR)

>            if hours >= 24:
>                days += 1
>                hours -= 24

This shouldn't be necessary because you already cut out the days

>    if seconds > mins:
>        while seconds > mins:
>            print "doing minutes"
>            seconds = seconds - mins
>            minutes += 1

minutes,seconds = divmod(seconds,MIN)

>            if minutes > 60:
>                hours += 1
>                minutes -= 60

Again it shoudn't be necessary but if it had been you would have
had to check the days again after increasing the hours...

>    secs = seconds

You don't need this, just return seconds...

>    return days, hours, minutes, secs
>
> production_time()

Just some ideas.

-- 
Alan Gauld
Author of the Learn to Program web site
http://www.alan-g.me.uk/




More information about the Tutor mailing list