4
\$\begingroup\$

My friend and I, have made a timetable viewer in Python 3.4 for a mock timetable. The format is a 2 week school timetable with 5 daily lessons with each lesson having a room. We have tried to make it as object-oriented as possible, yet is the solution overkill? Our goal was to make the classes into a module so you can just import timetable and create your own Lesson() and Day() objects to make your own timetable. So, is our solution overkill, and any general feedback on the code?

class Lesson:
    def __init__(self, name, room):
        self.room = room
        self.name = name
    def __repr__(self):
        return ('{}: {}').format(self.name, self.room)

class Day:
    def __init__(self, name, week, lessons):
        self.name = name
        self.week = week
        self.lessons = lessons
    def __repr__(self):
        return ((str([x for x in self.lessons]).replace('[', ' ')).replace(']', '')).replace(',', '\n')

physics = Lesson("Physics", "B27")
prod_des = Lesson("Product Design", "B2")
comp_sci = Lesson("Computer Science", "B14")
french = Lesson("French", "C9")
english = Lesson("English", "C11")
maths = Lesson("Maths", "T15")
private_study = Lesson("Private Study", "N/A") 
chem = Lesson("Chemistry", "B12")
engineering = Lesson("Engineering", "Off Campus") 
biol = Lesson("Biology", "B24")
add_math = Lesson("Additional Math", "T19")
games = Lesson("Games", "Sports Field")
re = Lesson("R.E.", "M20")
engineering_voc = Lesson("Engineering (Vocational)", "B2")

monday = Day("Monday", "Week 1",[physics, prod_des, comp_sci, french, english])
tuesday = Day("Tuesday", "Week 1", [english, maths, prod_des, physics, private_study])
wednesday = Day("Wednesday", "Week 1", [french, chem, comp_sci, engineering, engineering])
thursday = Day("Thursday", "Week 1", [french, private_study, biol, maths, english])
friday = Day("Friday", "Week 1", [comp_sci, prod_des, maths, add_math, games])

monday2 = Day("Monday", "Week 2",[maths, prod_des, biol, english, private_study])
tuesday2 = Day("Tuesday", "Week 2", [prod_des, french, chem, maths, private_study])
wednesday2 = Day("Wednesday", "Week 2", [comp_sci, physics, re, engineering, engineering])
thursday2 = Day("Thursday", "Week 2", [games, maths, biol, french, english])
friday2 = Day("Friday", "Week 2", [french, chem, engineering_voc, english, comp_sci])

days = { 1: monday, 2: tuesday, 3: wednesday, 4: thursday, 5: friday,
         6: monday2, 7: tuesday2, 8: wednesday2, 9: thursday2, 10: friday2
}

while True:
    while True:
        try:
            day = int(input("\nDay: "))
            if day in days.keys():
                day_obj = days[day]
                print(day_obj.name, day_obj.week)
                print(day_obj)
                break
            raise ValueError
        except:
            print("It has to be a number between 1 and 10")
    while True:
        again = input("Again? (y/n): ").lower()
        if again == "n":
            quit()
        elif again not in ["y", "n"]:
            print("Type 'y' or 'n'")
        elif again == "y":
            break
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Possible improvements:

  • Don't use __repr__ for a human readable version of the object. That is what __str__ is for. When writing your own __rerp__ you should aim to make eval(repr(obj)) work.
  • Rather than str.replace, you could str.format the values to the correct format. And so I'd change your Day.__repr__ to something more like:

    def __str__(self):
        return '\n'.join(' {!s}'.format(x) for x in self.lessons)
    
  • Rather than making two near duplicate loops, you could abstract the logic into one function, get_input. You could also make it so that it shows the user what input they are allowed to pick.

    This could be implemented as:

    def get_input(question, valid):
        question += '[{}] '.format('/'.join(valid))
        valid = set(valid)
        while True:
            answer = input(question).lower()
            if answer in valid:
                return answer
    
  • I'd change Day.__str__ so that it also displays the name and week.

  • I'd rather read a JSON file, than build the week structure in your program. This is as you may want to later build a GUI, and allow other students to enter their time-tables.

    I'll leave this down to the both of you to look into. :)

Excluding the last bullet point I'd change your code to:

class Lesson:
    def __init__(self, name, room):
        self.room = room
        self.name = name

    def __str__(self):
        return '{}: {}'.format(self.name, self.room)

class Day:
    def __init__(self, name, week, lessons):
        self.name = name
        self.week = week
        self.lessons = lessons

    def __str__(self):
        return '{} {}\n{}'.format(
            self.name,
            self.week,
            '\n'.join(' {!s}'.format(x) for x in self.lessons)
        )

def get_input(question, valid):
    question += '[{}] '.format('/'.join(valid))
    valid = set(valid)
    while True:
        answer = input(question).lower()
        if answer in valid:
            return answer

days_str = [str(day) for day in sorted(days.keys())]
while True:
    day = get_input('Day: ', days_str)
    print(days[int(day)])
    if 'n' == get_input('Again? ', ['y', 'n']):
        break
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.