Added enum instance for Date and date implementation of adjust#73
Conversation
|
@LiamGoodacre Could you maybe have a look at this as well? Really think this is missing and since the constructors are hidden it is cumbersome to implement outside of the library, not to mention fairly complicated. |
|
@LiamGoodacre cleaned it up and avoided some code duplication by using the |
|
@LiamGoodacre Are we good to go? |
|
@garyb Could you have a look here? Should be ready to merge. |
| in if n == 0 then fromJust (toEnum 7) else fromJust (toEnum n) | ||
|
|
||
| -- | Adjusts a date with a Duration in days. The day duration is | ||
| -- | converted to an Int using fromNumber. |
There was a problem hiding this comment.
My only comment in that case is let's explain the implication of fromNumber directly here, rather sending people off to have to look up what it means.
LiamGoodacre
left a comment
There was a problem hiding this comment.
I'm happy with this, thanks!
| in if n == 0 then fromJust (toEnum 7) else fromJust (toEnum n) | ||
|
|
||
| -- | Adjusts a date with a Duration in days. The number of days must | ||
| -- | fall within the valid range for an `Int` type otherwise `Nothing` |
There was a problem hiding this comment.
Let's be precise here and say:
The number must already be an integer and fall within the valid range of values for the Int type
because otherwise adjust (Days 3.1) someDate == Nothing would be confusing
There was a problem hiding this comment.
I have added your suggestion
This adds:
I initially implemented adjust without adding an enum instance for date, but implementation became quite complicated although possibly more efficient since it can recur with the number of days in a month rather than recur day by day. An enum instance turns out to be very useful.