-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: add alarms to express future time intent #13
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small things, like the new tests
} | ||
|
||
// Reset is part of the clock.Timer interface. | ||
func (a *alarm) Reset(t time.Time) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is alarm
really just for "implementing a mock clock.Alarm for testing purposes"?
It seems that resetTime
is all called here, so can't we just inline the code and avoid the extra func?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, the alarm implements a Reset that takes a time. The underlying timer implements everything else (but also implements a Reset that takes a duration).
|
||
// Alarm type represents a single event. | ||
// Alarms must be created with AtFunc or NewAlarm. | ||
type Alarm interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: This abstraction feels a bit weird: It is too close to Timer abstraction (with only Reset
signature as a differencies.
Stop()
isn't a correct abstraction, I think, since we don't "stop" an alarm. Stopping a timer has a sense since when restart it, we still have time to go.
suggestion: I think the right abstraction here is something like we have on our phone.
Timer
can be stopped and resumeAlarm
can be enabled or disabled. In future, they may become periodic (set an alarm everyday, at 5...
Reset
feels weird to me. Why having a reset timer when we can just throw up a new timer ?
However, I don't have the big picture, but with those names it is a little confusing (timer and alarm are implemented on our phone and follows well known standard. If our abstraction isn't that close of the alarm stuff on our phone, maybe our naming is confusing :D )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Desktop isn't a desk top, although we may borrow things, skeuomorphic abstractions do not need to be identical, only familiar.
In this case, it is important to give them different names, but functionally, they are very similar mechanisms. Same with the clock app on your phone: Alarms (alert at a point in time, with optional repeating), Timers (alert after a duration of elapsed time with a manual repeat), Stopwatch (a clock with a non standard epoch) and the Clock itself. An alarm is the closest abstraction we have.
For the Stop and Reset interface, I would argue, these are the same concept, they share abstractions, Go is the limiting factor here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree on the first point (abstraction - skeuomorphic or less exotic one :p ) don't need to be indentical, only familiar.
This is an interesting question to state if those are familiar. I stated that put in that way, Alarms and Timer feels weird to me (aka unfamiliar).
I understand the need (having something similar in functionality but with a different name). And I understand the choice of Alarm as a name, as explained by you.
However, my point of view, when I try to adopt "mundane" opinion, is that alarm and timer, besides the fact there are signal emitted over time, will evolves with slightly different path. And since its mundane opinion (or I believe it is), future developer may misunderstand the Alarm here (which is an AbsoluteTime Timer, where actual Timer is a RelativeTime timer, if we choose other sets of abstraction).
However that is only thoughts, so no worries, let's see ;) Thank you for your detailed answer. Agree to disagree :)
This expands on the concept of Alarms. Purely this helps only testing code.
Alarms are set to go off at or after a point in the future, rather than after an elapsed time duration.
This is useful for code to express the intent of its use of a time/alarm mechanism.
If a worker wants to wait a duration of time, it can do this by expressing a duration timer.
If a worker wants to wait until a point in time, it can do this by expressing a time alarm.
The distinction between the two is important when testing workers that deal with scheduled events rather than delayed events. An example is an expiration worker, it knows that a certificate will expire at a certain time in the future, so it schedules a timer for the delta between now and that time. With a wall clock, there is no difference between a timer and an alarm.
With testing clocks, they can move the Now point forward fast (cpu cycles fast), such that the point in time calculated is wrong:
With timers:
With alarms:
With both examples, the intent is to trigger the event at a specified time, calculation here is merely demonstration.