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

`plus` is not commutative #31

Open
p-himik opened this issue Apr 6, 2019 · 2 comments
Open

`plus` is not commutative #31

p-himik opened this issue Apr 6, 2019 · 2 comments
Labels

Comments

@p-himik
Copy link

@p-himik p-himik commented Apr 6, 2019

(jt/plus  (jt/zoned-date-time) (jt/minutes 1))
=> #inst"2019-04-06T10:35:04.615-00:00"
(jt/plus (jt/minutes 1) (jt/zoned-date-time))
Execution error (ClassCastException) at java-time.amount/d-plus (amount.clj:12).
class java.time.ZonedDateTime cannot be cast to class java.time.Duration (java.time.ZonedDateTime and java.time.Duration are in module java.base of loader 'bootstrap')
@dm3
Copy link
Owner

@dm3 dm3 commented Apr 8, 2019

Should plus be commutative though? What do you think should be the laws of algebra involving all of the date-times, intervals and periods?

Currently the API follows the Java object model where the date/time always comes first when a period/duration is added/subtracted. In Java this happens naturally due to the placement of methods. In Clojure we can do as we like. However, date-times and durations are different types - it's not obvious that operations including both should be commutative. Maybe the plus/minus naming could be changed to something like add-duration or extend to clear up the behaviour (although I quite like plus), what do you think?

@p-himik
Copy link
Author

@p-himik p-himik commented Apr 9, 2019

To be honest, I didn't really give it much thought so I cannot answer your questions properly. It's very well possible that the current state of affairs is the best one. It was just not really intuitive for me (at first, I even started searching in a completely different direction), and I didn't find any previous discussions on the matter.
If all is fine as it is, the issue can be closed and left just as a documentation for someone with a similar concern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.