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()