Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finished #871

Open
wants to merge 3 commits into
base: community
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions 52/zambam5/timer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import platform

from datetime import datetime, timedelta
from time import sleep

if platform.system() == "Windows":
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way of doing it, platform acnostic, is to just do the import and catch a ModuleNotFoundError with a try except

from winsound import Beep


def pomodoro_timer(duration: int = 20, beep: bool = False):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you are using type hints, annotate the whole function and that means also the return value.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better name for duration would be minutes in this case

"""
Take a duration and begin a timer loop
"""
duration_delta = timedelta(minutes=duration)
d = datetime.now()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable names should be speaking and have a meaning, this is not a good name :)

d_ = d + duration_delta
print("Starting timer " + str(d), "\r\nTimer will stop at " + str(d_))
sleep(duration_delta.seconds)
if beep:
Beep(frequency=500, duration=50)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is problematic because You do not check for the platform again, so this will fail ok all platforms where Beep is not available but beep is set to True

pass


def main():
if platform.system() == "Windows":
b_question = input(
'Do you want the timer to beep when finished? Answer "yes" or "no" '
)
if b_question == "no":
beep = False
else:
beep = True
else:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set beep to false before the user input and only set it to True when the user says yes, saves you one else branch

beep = False
while True:
pomodoro_timer(beep=beep)
again = input(
'Do you want to start the timer again? Answer "yes" or "no" only '
)
if again == "yes":
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have to check here, you can completely omit this branch, only check for when you want to do something. Here you want to do something if the user does not want to continue so only check for "no"

continue
else:
print("Ending the loop, hope you got some good work done!")
sleep(0.5)
break


if __name__ == "__main__":
main()