rluiten / elm-date-extra

Elm Date Extra library add/subtract/diff/format etc dates
http://package.elm-lang.org/packages/rluiten/elm-date-extra/latest
BSD 3-Clause "New" or "Revised" License
75 stars 36 forks source link

Period.add (Period.Delta (Period.diff d1 d2)) 1 d2 /= d1 #58

Closed rlefevre closed 6 years ago

rlefevre commented 6 years ago

Hi,

According to Period.diff documentation:

diff : Date -> Date -> DeltaRecord

Return a Period representing date difference. date1 - date2.

If you add the result of this function to date2 with addend of 1 will return date1.

but when performing the following test in elm-repl, I don't get date1 when adding Period.diff date1 date2 to date2:

> import Date
> import Date.Extra.Create as Date
> import Date.Extra.Period as Period

> d1 = Date.dateFromFields 2018 Date.Sep 29 0 0 0 0
<Sat Sep 29 2018 00:00:00 GMT+0200 (CEST)> : Date.Date

> d2 = Date.dateFromFields 2017 Date.Nov 2 0 0 0 0
<Thu Nov 02 2017 00:00:00 GMT+0100 (CET)> : Date.Date

> Period.diff d1 d2
{ week = 47, day = 1, hour = 0, minute = 0, second = 0, millisecond = 0 }
    : Date.Extra.Period.DeltaRecord

> Period.add (Period.Delta (Period.diff d1 d2)) 1 d2
<Fri Sep 28 2018 01:00:00 GMT+0200 (CEST)> : Date.Date

Did I miss something?

Thank you

rluiten commented 6 years ago

Period is for doing math for elapsed time units only. It does not compensate for how time varies due to day light saving, varying month lengths and varying year lengths.

For stuff handling years / months and day light saving use

I created an ellie example with Period code and with Duration code and it appears to work for Duration for me.

https://ellie-app.com/h7V7Zjw8Ya1/2

Open the javascript console in your browser to see logged versions of diff, and durationDiff identifiers.

I setup anotebook here to UTC+2.0 Damascaus time zone and the output to html was.

d1:2018-09-29T00:00:00.000+03:00 
d2:2017-11-02T00:00:00.000+02:00 
d3:2018-09-28T01:00:00.000+03:00 
d3Duration:2018-09-29T00:00:00.000+03:00

And the javascript console.log was

diff: { week = 47, day = 1, hour = 0, minute = 0, second = 0, millisecond = 0 }
durationDiff: { year = 0, month = 10, day = 27, hour = 0, minute = 0, second = 0, millisecond = 0 }

Using Duration fixes this problem I believe feel free to correct me if I have missed something here.

rlefevre commented 6 years ago

I know about Duration. What seems wrong to me is the Period.diff documentation saying:

If you add the result of this function to date2 with addend of 1 will return date1.

That does not seem to be the case to me, at least not for my test. Do I understand incorrectly the documentation?

rluiten commented 6 years ago

You are correct, I do need to update the documentation, for human based time representations as in our standard calendar it is not a correct statement for Period.

rlefevre commented 6 years ago

Thank you. This clears things up.

rluiten commented 6 years ago

I have updated the doc error, and added a bit to try make the difference clearer in 9.2.1. Thanks for your input @rlefevre .