Timer convenience class to time code execution for user output

Question

I implemented a convenience class to time code execution and print it to the user. This is part of a bigger project that will be distributed to external users. The timer however is only called by other procedures, not by the user directly. I was interested in implementing this myself, so I'm not looking for an external module or similiar. Docstrings are omitted here, the functions are rather straightforward.

from time import perf_counter


class Timer:
    def __init__(self, default_round_digits: int = 5):
        self.default_digits = default_round_digits
        self.start_timer()

    def start_timer(self) -> None:
        self.start_time = perf_counter()
        self.pause_start_time = None
        self.pause_length = 0

    def pause_timer(self) -> None:
        self.pause_start_time = perf_counter()

    def resume_timer(self) -> None:
        if self.pause_start_time is not None:
            self.pause_length += perf_counter() - self.pause_start_time
            self.pause_start_time = None

    @property
    def elapsed_seconds(self) -> float:
        # If timer is paused only consider time up to self.pause_start_time instead of now
        if self.pause_start_time is not None:
            return self.pause_start_time - self.start_time - self.pause_length
        else:
            return perf_counter() - self.start_time - self.pause_length

    def get_timer(self, round_digits: int = None, print_text: str = None, restart: bool = False) -> float:

        if round_digits is None:
            round_digits = self.default_digits

        elapsed_seconds = round(self.elapsed_seconds, ndigits=round_digits)

        if print_text is not None:
            print(f"{print_text}{elapsed_seconds} seconds")

        if restart:
            self.start_timer()

        return elapsed_seconds

    def __str__(self) -> str:
        state = "Running" if self.pause_start_time is None else "Paused"
        return f"{state} Timer at {self.get_timer()} seconds"

I'm still unsure about initializing attributes outside of __init__. I know it's not recommended, but copying start_timer into __init__ seemed like the worse option to me.

Answer 1

1. Your Timer class contains two timers.

   1. The entire duration timer.
   2. The timer for tracking pauses.

   I'd move the core timer code out into a separate class. And we can see you're starting to repeat yourself.

2. Initializing a timer shouldn't start the timer. If I open my timer app should the timer start running straight away?

   If I want to create some timers but I don't want the timer to be started your class gives me no options. However just not calling start_timer in __init__ will allow everyone to use your class however is desired.

   If you really really want to start the timer when building the object, you can write a class method.

   class Timer:
       @classmethod
       def run(cls, *args, **kwargs):
           self = cls(*args, **kwargs)
           self.start_timer()
           return self
   
   # option 1
   timer = Timer()
   timer.start_timer()
   
   # option 2
   timer = Timer.run()
   

3. start_timer restarts the timers. You should rename the method or make the method do what is expected.

4. If you call pause_timer twice without calling resume_timer your code acts as if the timer was never paused the first time.

5. I don't really see the point of making elapsed_seconds a property.

6. get_timer is doing way too much and violates the SRP principle.

   1. Rounds elapsed timings to a specified amount of decimal places.
   2. Prints the elapsed timings.
   3. Restarts the timer.
   4. Returns the elapsed timings.

   You should make a function to round the elapsed timings. Then leave printing and restarting to the user.

7. I'd a couple more methods from functionality I'd expect from a timer.

from time import perf_counter


class CoreTimer:
    def __init__(self):
        self.stop()

    def __bool__(self):
        return bool(self.start_time)

    def start(self):
        self.start_time = perf_counter()

    def stop(self):
        self.start_time = 0

    def time_taken(self, now):
        return now - self.start_time

    def elapsed(self):
        return self.time_taken(perf_counter())


class Timer:
    def __init__(self, decimal_places=5):
        self._decimal_places = decimal_places
        self._timer = CoreTimer()
        self._pause = CoreTimer()
        self._pause_length = 0

    def __str__(self):
        state = "Paused" if self._pause else "Running"
        return f"{state} Timer at {self.elapsed_rounded()} seconds"

    @classmethod
    def run(cls, *args, **kwargs):
        self = cls(*args, **kwargs)
        self.start()
        return self

    def start(self):
        if not self._timer:
            self._timer.start()

    def stop(self):
        self._timer.stop()
        self._pause.stop()
        self._pause_length = 0

    def restart(self):
        self.stop()
        self.start()

    def pause(self):
        if not self._pause:
            self._pause.start()

    def resume(self):
        if self._pause:
            self._pause_length += self._pause.elapsed()
            self._pause.stop()

    def elapsed(self):
        if self._pause:
            duration = self._timer.time_taken(self._pause.start_time)
        else:
            duration = self._timer.elapsed()
        return duration - self._pause_length

    def elapsed_rounded(self, decimal_places=None):
        if decimal_places is None:
            decimal_places = self.decimal_places
        return round(self.elapsed(), ndigits=decimal_places)

Answer 2

Regarding your question about attribute initialization:

It's not necessarily better to put all attributes into __init__. It's generally recommended, since we expect to find all class attributes inside of __init__, but I would argue in this case, it's just as readable. In fact I would say initializing these values with None inside __init__ (just to suppress the warnings or follow the recommendation) hurts readability in your case:

def __init__(self, default_round_digits: int = 5):
    self.default_digits = default_round_digits

    self.start_time = None
    self.pause_start_time = None
    self.pause_length = None

    self.start_timer()

Here are some further discussions on StackOverflow regarding the topic:

• Instance attribute attribute_name defined outside init
• Python - instance attribute defined outside init()
• why is defining an object variable outside of init frowned upon?