andrewmcveigh / cljs-time

A clj-time inspired date library for clojurescript.
342 stars 57 forks source link

Missing IHash implementation #58

Closed smee closed 8 years ago

smee commented 8 years ago

I have a usecase where I need to keep dates as keys in a map. When trying to find an entry I sometimes get nil even though the date is present (as indicated by cljs-time.core/=). Sadly, I can't reproduce the behaviour in the repl easily (I receive the map through transit, reading the dates via (goog.date.UtcDateTime.fromIsoString s)). Everything works if I implement IHash:

(extend-type goog.date.UtcDateTime IHash (-hash [this] (cljs-time.coerce/to-long this)))
(extend-type goog.date.Date IHash (-hash [this] (cljs-time.coerce/to-long this)))
(extend-type goog.date.DateTime IHash (-hash [this] (cljs-time.coerce/to-long this)))

Issue #19 talked about this, but the implementation in cljs-time.extends just focuses on IEquiv and IComparable, leaving out IHash.

andrewmcveigh commented 8 years ago

Hi,

So I can't remember if there was a reason I didn't implement IHash, maybe I just overlooked it. I don't think that cljs-time.coerce/to-long is quite suitable, as the different types Date, DateTime, and UtcDateTime shouldn't hash the same.

I'll probably take the way joda-time does it.

hipitihop commented 8 years ago

@andrewmcveigh Are you planning a release soon which includes this ?

andrewmcveigh commented 8 years ago

@hipitihop Hey, sorry about the delay. I just pushed a 0.5.0-alpha1 as I'm a little hesitant to release 0.5.0 proper yet.