Skip to main content
deleted 9 characters in body
Source Link
Daniel
  • 4.6k
  • 2
  • 18
  • 40

Very clean code indeed, there's not much to be said here.

PEP8

I'm sure you've been lectured on this before, but there's an official style guide for Python called PEP8. It states that module-level class definitions should be separated by two lines:

import tkinter as tk
import datetime


class Countdown(tk.Frame):
    ...

You also leave the first line of every method blank (or make it a docstring). If a function is undocumented, I wouldn't bother adding a blank line:

def foo():
    do_stuff()

If you decide to add a docstring, I'd recommend the following structure:

< function definition >
    < docstring (may span multiple lines) >
    < blank line >
    < function body >

For example:

def bar():
    """I'm a docstring!
    Blah, blah.
    """

    do_stuff()

Private attributes

It's good that you marked _timer_on as private. Why not do the same for seconds_left and the helper methods? This clearly tells other developers: 'I'm not part of the API. Don't rely on me for backwards compatibility.'. (Added bonus: you don't have to document private functions).

class Countdown(tk.Frame):
    ...

    def show_widgets(self):
        < docstring >

        ...

    def create_widgets(self):
        < docstring >

        ...

    def _countdown(self):
        ...

    def _start_button(self):
        ...

    def _stop_timer(self):
        ...

    def _convert_seconds_left_to_time(self):
        ...

Basically, any method, property, or attribute that isn't meaningfuluseful for the public APIend users, should be private.

OOP

convert_seconds_left_to_time could be a staticmethod. A static method does not receive anyan implicit first argument. You should make a method static if it doesn't interact with the instance of the class, but is still strongly related to the class.

I'd then rename it to _get_timedelta_from_seconds:

class Countdown(tk.Frame):
    ...

    def countdown(self):
        """Update the label based on the time left."""

        self.label["text"] = self._get_timedelta_from_seconds(self.seconds_left)

    @staticmethod
    def _get_timedelta_from_seconds(seconds):
        return datetime.timedelta(seconds=seconds)

Very clean code indeed, there's not much to be said here.

PEP8

I'm sure you've been lectured on this before, but there's an official style guide for Python called PEP8. It states that module-level class definitions should be separated by two lines:

import tkinter as tk
import datetime


class Countdown(tk.Frame):
    ...

You also leave the first line of every method blank (or make it a docstring). If a function is undocumented, I wouldn't bother adding a blank line:

def foo():
    do_stuff()

If you decide to add a docstring, I'd recommend the following structure:

< function definition >
    < docstring (may span multiple lines) >
    < blank line >
    < function body >

For example:

def bar():
    """I'm a docstring!
    Blah, blah.
    """

    do_stuff()

Private attributes

It's good that you marked _timer_on as private. Why not do the same for seconds_left and the helper methods? This clearly tells other developers: 'I'm not part of the API. Don't rely on me for backwards compatibility.'. (Added bonus: you don't have to document private functions).

class Countdown(tk.Frame):
    ...

    def show_widgets(self):
        < docstring >

        ...

    def create_widgets(self):
        < docstring >

        ...

    def _countdown(self):
        ...

    def _start_button(self):
        ...

    def _stop_timer(self):
        ...

    def _convert_seconds_left_to_time(self):
        ...

Basically, any method, property or attribute that isn't meaningful for the public API, should be private.

OOP

convert_seconds_left_to_time could be a staticmethod. A static method does not receive any implicit first argument. You should make a method static if it doesn't interact with the instance of the class, but is still strongly related to the class.

I'd then rename it to _get_timedelta_from_seconds:

class Countdown(tk.Frame):
    ...

    def countdown(self):
        """Update the label based on the time left."""

        self.label["text"] = self._get_timedelta_from_seconds(self.seconds_left)

    @staticmethod
    def _get_timedelta_from_seconds(seconds):
        return datetime.timedelta(seconds=seconds)

Very clean code indeed, there's not much to be said here.

PEP8

I'm sure you've been lectured on this before, but there's an official style guide for Python called PEP8. It states that module-level class definitions should be separated by two lines:

import tkinter as tk
import datetime


class Countdown(tk.Frame):
    ...

You also leave the first line of every method blank (or make it a docstring). If a function is undocumented, I wouldn't bother adding a blank line:

def foo():
    do_stuff()

If you decide to add a docstring, I'd recommend the following structure:

< function definition >
    < docstring (may span multiple lines) >
    < blank line >
    < function body >

For example:

def bar():
    """I'm a docstring!
    Blah, blah.
    """

    do_stuff()

Private attributes

It's good that you marked _timer_on as private. Why not do the same for seconds_left and the helper methods? This clearly tells other developers: 'I'm not part of the API. Don't rely on me for backwards compatibility.'. (Added bonus: you don't have to document private functions).

class Countdown(tk.Frame):
    ...

    def show_widgets(self):
        < docstring >

        ...

    def create_widgets(self):
        < docstring >

        ...

    def _countdown(self):
        ...

    def _start_button(self):
        ...

    def _stop_timer(self):
        ...

    def _convert_seconds_left_to_time(self):
        ...

Basically, any method, property, or attribute that isn't useful for end users, should be private.

OOP

convert_seconds_left_to_time could be a staticmethod. A static method does not receive an implicit first argument. You should make a method static if it doesn't interact with the instance of the class, but is still strongly related to the class.

I'd then rename it to _get_timedelta_from_seconds:

class Countdown(tk.Frame):
    ...

    def countdown(self):
        """Update the label based on the time left."""

        self.label["text"] = self._get_timedelta_from_seconds(self.seconds_left)

    @staticmethod
    def _get_timedelta_from_seconds(seconds):
        return datetime.timedelta(seconds=seconds)
Indentation
Source Link
Daniel
  • 4.6k
  • 2
  • 18
  • 40

Very clean code indeed, there's not much to be said here.

PEP8

I'm sure you've been lectured on this before, but there's an official style guide for Python called PEP8. It states that module-level class definitions should be separated by two lines:

import tkinter as tk
import datetime


class Countdown(tk.Frame):
    ...

You also leave the first line of every method blank (or make it a docstring). If a function is undocumented, I wouldn't bother adding a blank line:

def foo():
    do_stuff()

If you decide to add a docstring, I'd recommend the following structure:

< function definition >
    < docstring (may span multiple lines) >
    < blank line >
    < function body >

For example:

def bar():
    """I'm a docstring!
    Blah, blah.
    """

    do_stuff()

Private attributes

It's good that you marked _timer_on as private. Why not do the same for seconds_left and the helper methods? This clearly tells other developers: 'I'm not part of the API. Don't rely on me for backwards compatibility.'. (Added bonus: you don't have to document private functions).

class Countdown(tk.Frame):
    ...

    def show_widgets(self):
        < docstring >

        ...

    def create_widgets(self):
        < docstring >

        ...

    def _countdown(self):
        ...

    def _start_button(self):
        ...

    def _stop_timer(self):
        ...

    def _convert_seconds_left_to_time(self):
        ...

Basically, any method, property or attribute that isn't meaningful for the public API, should be private.

OOP

convert_seconds_left_to_time could be a staticmethod. A static method does not receive any implicit first argument. You should make a method static if it doesn't interact with the instance of the class, but is still strongly related to the class.

I'd then rename it to _get_timedelta_from_seconds:

class Countdown(tk.Frame):
    ...

    def countdown(self):
        """Update the label based on the time left."""

        self.label["text"] = self._get_timedelta_from_seconds(self.seconds_left)

    @staticmethod
    def _get_timedelta_from_seconds(seconds):
        return datetime.timedelta(seconds=seconds)

Very clean code indeed, there's not much to be said here.

PEP8

I'm sure you've been lectured on this before, but there's an official style guide for Python called PEP8. It states that module-level class definitions should be separated by two lines:

import tkinter as tk
import datetime


class Countdown(tk.Frame):
    ...

You also leave the first line of every method blank (or make it a docstring). If a function is undocumented, I wouldn't bother adding a blank line:

def foo():
    do_stuff()

If you decide to add a docstring, I'd recommend the following structure:

< function definition >
    < docstring (may span multiple lines) >
    < blank line >
    < function body >

For example:

def bar():
    """I'm a docstring!
    Blah, blah.
    """

    do_stuff()

Private attributes

It's good that you marked _timer_on as private. Why not do the same for seconds_left and the helper methods? This clearly tells other developers: 'I'm not part of the API. Don't rely on me for backwards compatibility.'. (Added bonus: you don't have to document private functions).

class Countdown(tk.Frame):
    ...

    def show_widgets(self):
        < docstring >

        ...

    def create_widgets(self):
        < docstring >

        ...

    def _countdown(self):
        ...

    def _start_button(self):
        ...

    def _stop_timer(self):
        ...

    def _convert_seconds_left_to_time(self):
        ...

Basically, any method, property or attribute that isn't meaningful for the public API, should be private.

OOP

convert_seconds_left_to_time could be a staticmethod. A static method does not receive any implicit first argument. You should make a method static if it doesn't interact with the instance of the class, but is still strongly related to the class.

I'd then rename it to _get_timedelta_from_seconds:

class Countdown(tk.Frame):
    ...

def countdown(self):
    """Update the label based on the time left."""

    self.label["text"] = self._get_timedelta_from_seconds(self.seconds_left)

@staticmethod
def _get_timedelta_from_seconds(seconds):
    return datetime.timedelta(seconds=seconds)

Very clean code indeed, there's not much to be said here.

PEP8

I'm sure you've been lectured on this before, but there's an official style guide for Python called PEP8. It states that module-level class definitions should be separated by two lines:

import tkinter as tk
import datetime


class Countdown(tk.Frame):
    ...

You also leave the first line of every method blank (or make it a docstring). If a function is undocumented, I wouldn't bother adding a blank line:

def foo():
    do_stuff()

If you decide to add a docstring, I'd recommend the following structure:

< function definition >
    < docstring (may span multiple lines) >
    < blank line >
    < function body >

For example:

def bar():
    """I'm a docstring!
    Blah, blah.
    """

    do_stuff()

Private attributes

It's good that you marked _timer_on as private. Why not do the same for seconds_left and the helper methods? This clearly tells other developers: 'I'm not part of the API. Don't rely on me for backwards compatibility.'. (Added bonus: you don't have to document private functions).

class Countdown(tk.Frame):
    ...

    def show_widgets(self):
        < docstring >

        ...

    def create_widgets(self):
        < docstring >

        ...

    def _countdown(self):
        ...

    def _start_button(self):
        ...

    def _stop_timer(self):
        ...

    def _convert_seconds_left_to_time(self):
        ...

Basically, any method, property or attribute that isn't meaningful for the public API, should be private.

OOP

convert_seconds_left_to_time could be a staticmethod. A static method does not receive any implicit first argument. You should make a method static if it doesn't interact with the instance of the class, but is still strongly related to the class.

I'd then rename it to _get_timedelta_from_seconds:

class Countdown(tk.Frame):
    ...

    def countdown(self):
        """Update the label based on the time left."""

        self.label["text"] = self._get_timedelta_from_seconds(self.seconds_left)

    @staticmethod
    def _get_timedelta_from_seconds(seconds):
        return datetime.timedelta(seconds=seconds)
Source Link
Daniel
  • 4.6k
  • 2
  • 18
  • 40

Very clean code indeed, there's not much to be said here.

PEP8

I'm sure you've been lectured on this before, but there's an official style guide for Python called PEP8. It states that module-level class definitions should be separated by two lines:

import tkinter as tk
import datetime


class Countdown(tk.Frame):
    ...

You also leave the first line of every method blank (or make it a docstring). If a function is undocumented, I wouldn't bother adding a blank line:

def foo():
    do_stuff()

If you decide to add a docstring, I'd recommend the following structure:

< function definition >
    < docstring (may span multiple lines) >
    < blank line >
    < function body >

For example:

def bar():
    """I'm a docstring!
    Blah, blah.
    """

    do_stuff()

Private attributes

It's good that you marked _timer_on as private. Why not do the same for seconds_left and the helper methods? This clearly tells other developers: 'I'm not part of the API. Don't rely on me for backwards compatibility.'. (Added bonus: you don't have to document private functions).

class Countdown(tk.Frame):
    ...

    def show_widgets(self):
        < docstring >

        ...

    def create_widgets(self):
        < docstring >

        ...

    def _countdown(self):
        ...

    def _start_button(self):
        ...

    def _stop_timer(self):
        ...

    def _convert_seconds_left_to_time(self):
        ...

Basically, any method, property or attribute that isn't meaningful for the public API, should be private.

OOP

convert_seconds_left_to_time could be a staticmethod. A static method does not receive any implicit first argument. You should make a method static if it doesn't interact with the instance of the class, but is still strongly related to the class.

I'd then rename it to _get_timedelta_from_seconds:

class Countdown(tk.Frame):
    ...

def countdown(self):
    """Update the label based on the time left."""

    self.label["text"] = self._get_timedelta_from_seconds(self.seconds_left)

@staticmethod
def _get_timedelta_from_seconds(seconds):
    return datetime.timedelta(seconds=seconds)