HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Repetitive days of the week code

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
therepetitiveweekcodedays

Problem

Could someone help me shorten this code, please?

while cd == "monday":
    if cp == "0":
        for v in mon.values():
            print(v)
            break
        break
    elif cp == "1":
        print(mon[1])
        break
    elif cp == "2":
        print(mon[2])
        break
    elif cp == "3":
        print(mon[3])
        break
    elif cp == "4":
        print(mon[4])
        break
    elif cp == "5":
        print(mon[5])
        break
    cd = False

while cd == "tuesday":
    if cp == "0":
        for v in tues.values():
            print(v)
            break
        break
    elif cp == "1":
        print(tues[1])
        break
    elif cp == "2":
        print(tues[2])
        break
    elif cp == "3":
        print(tues[3])
        break
    elif cp == "4":
        print(tues[4])
        break
    elif cp == "5":
        print(tues[5])
        break
    cd = False

Solution

You could simplify your code to look something like this (warning: I didn't test my code, so you should double-check to make sure it functions identically to the original)

days_map = {
    "monday": mon,
    "tuesday": tues
}

day = days_map[cd]
if cp == "0":
    print(day.values()[0])
else:
    print(day[int(cp)])


Several notes:

In your code, you use a while loop, but in every case, immediately break out of it, either by literally using break or by making the conditional false. What you want instead is an "if" statement. However, because the "if" statements are almost identical, you can simplify it completely by replacing it with a call to a dict.

(In case you aren't familiar with what dicts are, they're really nifty data structures that make life insanely easier. As you can see, they helped turn a large chunk of code into just 10 or so lines! Here's a link to CodeAcademy which gives an introduction to them))

I'm assuming that mon and tues are dicts of some kind -- I made another dict (days_map) to refer to them. This is a common technique in Python -- whenever you have a large amount of if and elif statements, check to see if you can somehow fold things into a dictionary so you can need to make only a single call.

Your for loop is also redundant. If you have a break immediately after, then it's equivalent to simply printing the first value.

Code Snippets

days_map = {
    "monday": mon,
    "tuesday": tues
}

day = days_map[cd]
if cp == "0":
    print(day.values()[0])
else:
    print(day[int(cp)])

Context

StackExchange Code Review Q#31818, answer score: 7

Revisions (0)

No revisions yet.