-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add Time#shift for changing a time instance by calendar units #6598
Add Time#shift for changing a time instance by calendar units #6598
Conversation
I don't understand the difference between the 2 methods (if there is one), could you give an example showing the difference? Ty |
If the wall clock advances by two hours, it shows a time two hours later, but that doesn't mean the elapsed time between both instants also equals to two hours. For example when there is a daylight savings switch in between, they might be apart on the instant time line by the duration of only one hour or even three hours. |
@straight-shoota first off, let me say thank you for tackling so much of the nonsense of time and date management. This is one of the things that can create a quality legacy in a language/stdlib and it can be so helpful. I believe you're trying to capture the essence of what it means to modify a timestamp by measured time, instead of simply incrementing the digits timestamp as if it was simple math, which I believe is what If that is the case, the word Either way, thanks for this. |
@straight-shoota needs a rebase |
Had a joke/suggestion to rename it to |
b5dc65c
to
e6a6d6c
Compare
@robacarp Yes, |
@luislavena I think the time travel metaphor fits better for manipulating the clock (such as timecop.cr). |
bump |
e6a6d6c
to
50a2da8
Compare
ping @bcardiff |
50a2da8
to
88b3a90
Compare
ed26a0e
to
c636933
Compare
c636933
to
8686174
Compare
Rebased and ready to ship. |
Based on #6574, only the last two commits are specific to this PR.
The main part of this PR renames
Time#add_months
toTime#shift
and adds arguments for additional calendrical units.This is copied from Timex.shift/2. An also fitting alternative would be
#advance
(from Rails' Time.advance).This method can be used for adding arbitrary amounts of (conceptual) time units and forms a basis for an upcoming
Time::DateSpan
type (see #6522).This is partly discussed in #6522 and #4556
Regarding performance, the new
#shift
is about twice as fast as#add_months
for shifting by months/years. The comparison is not really fair though, because#add_months
calledyear
,month
andday
individually which means the rather expensive calculations inyear_month_day_day_year
are executed three times instead of once. With this simple optimization applied,#shift
is still 50% faster than#add_months
(probably because it does't go through the calendrical constructor).Time#add_span
is also renamed toTime#shift
which is a more fitting name as well and means there are two implementations ofTime#shift
, one based on elapsed time in seconds, the other on calendrical units.So now there are two ways for adding a time span to an instance of
Time
:time + 2.hours
advances by the duration of two hours on the instant time-line (elapsed time) andtime.shift hours: 2
advances by two hours in local time (wall clock).