Alarm clock subclass of a clock

Question

I have these two classes and they should remain separate classes, but I have a feeling that this could be implemented more efficiently and with less duplicate code. After googling around I still didn't figure a better way, so perhaps someone here could point me out. Ideally I want to get tick() method from Clock and just add alarm to it and also get set_time() method with additional attributes for setting the alarm time.

class Clock:

def __init__(self, hours, minutes, seconds):
    self.set_time(hours, minutes, seconds)

def set_time(self, hours, minutes, seconds):
    if hours < 0:
        self.hours = 0
    elif hours > 23:
        self.hours = 23
    else:
        self.hours = hours

    if minutes < 0:
        self.minutes = 0
    elif minutes > 59:
        self.minutes = 59
    else:
        self.minutes = minutes

    if seconds < 0:
        self.seconds = 0
    elif seconds > 59:
        self.seconds = 59
    else:
        self.seconds = seconds

def tick(self):
    while True:
        print(str(self.hours).zfill(2), ":", str(self.minutes).zfill(2), ":", str(self.seconds).zfill(2), sep="")
        self.seconds += 1
        time.sleep(1)
        if self.seconds == 60:
            self.seconds = 0
            self.minutes += 1
            if self.minutes == 60:
                self.minutes = 0
                self.hours += 1
                if self.hours == 24:
                    self.hours = 0


class AlarmClock(Clock):

def __init__(self, hours, minutes, seconds, alarm_hr, alarm_min, alarm_sec):
    super().__init__(hours, minutes, seconds)
    self.set_alarm(alarm_hr, alarm_min, alarm_sec)

def set_alarm(self, alarm_hr, alarm_min, alarm_sec):
    if alarm_hr < 0:
        self.alarm_hr = 0
    elif alarm_hr > 23:
        self.alarm_hr = 23
    else:
        self.alarm_hr = alarm_hr

    if alarm_min < 0:
        self.alarm_min = 0
    elif alarm_min > 59:
        self.alarm_min = 59
    else:
        self.alarm_min = alarm_min

    if alarm_sec < 0:
        self.alarm_sec = 0
    elif alarm_sec > 59:
        self.alarm_sec = 59
    else:
        self.alarm_sec = alarm_sec

def alarm(self):
    while True:
        print(str(self.hours).zfill(2), ":", str(self.minutes).zfill(2), ":", str(self.seconds).zfill(2), sep="")
        self.seconds += 1
        time.sleep(1)
        if self.seconds == 60:
            self.seconds = 0
            self.minutes += 1
            if self.minutes == 60:
                self.minutes = 0
                self.hours += 1
                if self.hours == 24:
                    self.hours = 0
        if self.hours == self.alarm_hr and self.minutes == self.alarm_min and self.seconds == self.alarm_sec:
            print("ALARM ALARM ALARM ALARM ALARM!!!!")
            break

Answer 1

I have refactored some of your code, please note that I am ignoring pre-existing libraries since I assume there's a particular reason (educational or otherwise) behind implementing Clock, tick, etc. this way on your own.

class Clock:

class Clock:
    def __init__(self, hours: int, minutes: int, seconds: int):
        self.hours = self.normalize_time_in_range(hours, 0, 23)
        self.minutes = self.normalize_time_in_range(minutes, 0, 59)
        self.seconds = self.normalize_time_in_range(seconds, 0, 59)

    @staticmethod
    def normalize_time_in_range(value: int, lower_bound: int, upper_bound: int) -> int:
        if value < lower_bound:
            return lower_bound
        elif value > upper_bound:
            return upper_bound
        else:
            return value

    def start_ticking(self):
        while True:
            self.tick()

    def tick(self):
        print(f"{str(self.hours).zfill(2)}:{str(self.minutes).zfill(2)}:{str(self.seconds).zfill(2)}")
        self.seconds += 1
        time.sleep(1)
        if self.seconds == 60:
            self.seconds = 0
            self.minutes += 1
            if self.minutes == 60:
                self.minutes = 0
                self.hours += 1
                if self.hours == 24:
                    self.hours = 0

1. normalize_value_in_range: Notice you have a lot of duplicated logic in set_time and set_alarm. Most of the logic can be extracted to this more generic implementation.
2. start_ticking and tick: In your case I found it useful to seperate the loop from the actual tick logic so you can change the loop conditions in subclasses while reusing tick. You will see why this is useful in AlarmClock.
3. Type annotations: As you can see in the method heads I added some type hints to the newly added methods. This is generally a good idea in regards to documenting your code for yourself and others.

class AlarmClock:

class AlarmClock(Clock):
    def __init__(self, hours, minutes, seconds, alarm_hr, alarm_min, alarm_sec):
        super().__init__(hours, minutes, seconds)

        self.alarm_hr = self.normalize_value_in_range(alarm_hr, 0, 23)
        self.alarm_min = self.normalize_value_in_range(alarm_min, 0, 59)
        self.alarm_sec = self.normalize_value_in_range(alarm_sec, 0, 59)

        self.ticking = False

    def start_ticking(self):
        self.ticking = True
        while self.ticking:
            self.tick()

    def tick(self):
        super().tick()
        if self.hours == self.alarm_hr and self.minutes == self.alarm_min and self.seconds == self.alarm_sec:
            print("ALARM ALARM ALARM ALARM ALARM!!!!")
            self.ticking = False

1. normalize_value_in_range: We can now reuse this method to set our alarm time as well.
2. start_ticking and tick: Since we seperated the loop from the actual tick we can use Clock.tick() in our AlarmClock without the need for duplicate code. Adding the attribute self.ticking to AlarmClock allows us to break out of the loop across methods. The reason we could not reuse tick() before was because it contained the infinite while True loop, thereby not allowing us to intercept when the alarm_time is reached.

class Time: Lastly, I would suggest seperating time logic from clock logic. This makes the clocks more concise and allows you to extend the functionality of the Time class as needed. You could also say it allows the clocks to focus on their main functionality. Again, I am ignoring the existence of libraries for this task, since I presume you want to implement the entire logic on your own. Specialized libraries like datetime are worth looking into since they provide a lot of comfortable functionality. One implementation might be the following:

class Time:
    def __init__(self, hours: int, minutes: int, seconds: int):
        self.hours = self.normalize_value_in_range(hours, 0, 23)
        self.minutes = self.normalize_value_in_range(minutes, 0, 59)
        self.seconds = self.normalize_value_in_range(seconds, 0, 59)

    @staticmethod
    def normalize_value_in_range(value: int, lower_bound: int, upper_bound: int) -> int:
        if value < lower_bound:
            return lower_bound
        elif value > upper_bound:
            return upper_bound
        else:
            return value

    def increment(self):
        self.seconds += 1
        if self.seconds == 60:
            self.seconds = 0
            self.minutes += 1
            if self.minutes == 60:
                self.minutes = 0
                self.hours += 1
                if self.hours == 24:
                    self.hours = 0

    def __eq__(self, other):
        return self.hours == other.hours and self.minutes == other.minutes and self.seconds == other.seconds

    def __str__(self):
        return f"{str(self.hours).zfill(2)}:{str(self.minutes).zfill(2)}:{str(self.seconds).zfill(2)}"


class Clock:
    def __init__(self, hours: int, minutes: int, seconds: int):
        self.time = Time(hours, minutes, seconds)

    def start_ticking(self):
        while True:
            self.tick()

    def tick(self):
        print(self.time)
        self.time.increment()
        time.sleep(1)


class AlarmClock(Clock):
    def __init__(self, hours, minutes, seconds, alarm_hr, alarm_min, alarm_sec):
        super().__init__(hours, minutes, seconds)
        self.alarm_time = Time(alarm_hr, alarm_min, alarm_sec)

        self.ticking = False

    def start_ticking(self):
        self.ticking = True
        while self.ticking:
            self.tick()

    def tick(self):
        super().tick()
        if self.time == self.alarm_time:
            print("ALARM ALARM ALARM ALARM ALARM!!!!")
            self.ticking = False

Answer 2

Your project provides a good illustration of a general principle in writing software: internal data storage (for example, how a Clock class keeps track of the time) and external presentation (for example, how a time should be printed for a user) are two different things.

Your code can be greatly simplified by storing the data in the most basic format possible: total seconds. Only when needed for the benefit of users do you worry about assembling a time in HH:MM:SS format. If we take this approach, you'll need a method to convert hours, minutes, seconds to total seconds. And we also need a way to convert the other direction. For brevity, I'm ignoring validation here:

import time

class Clock:

    def __init__(self, hours, minutes, seconds):
        # Just store total seconds.
        self.total_secs = self.hms_to_seconds(hours, minutes, seconds)

    def hms_to_seconds(self, hours, minutes, seconds):
        # Simple converter.
        return hours * 3600 + minutes * 60 + seconds

    # Properties to retrieve user-facing values when needed.

    @property
    def hours(self):
        return self.total_secs // 3600

    @property
    def minutes(self):
        return (self.total_secs - (3600 * self.hours)) // 60

    @property
    def seconds(self):
        return self.total_secs - 3600 * self.hours - 60 * self.minutes

    def tick(self):
        # Make tick do less, so you can re-use it.
        print(f'{self.hours:>02}:{self.minutes:>02}:{self.seconds:>02}')
        self.total_secs += 1
        time.sleep(1)

    def run(self):
        # A basic clock just ticks forever.
        while True:
            self.tick()

class AlarmClock(Clock):

    def __init__(self, hours, minutes, seconds, alarm_hr, alarm_min, alarm_sec):
        super().__init__(hours, minutes, seconds)
        self.alarm_secs = self.hms_to_seconds(alarm_hr, alarm_min, alarm_sec)

    def run(self):
        # An alarm clock ticks and alarms.
        while True:
            self.tick()
            if self.total_secs >= self.alarm_secs:
                print("ALARM ALARM ALARM ALARM ALARM!!!!")
                break

AlarmClock(1, 15, 0, 1, 15, 5).run()