From aa1ac992de618d0a91e40871e3577ba72f128c46 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Wed, 9 May 2018 08:57:27 -0700 Subject: [PATCH] Small fixes for ordered map equiv and equals methods There are test cases added that fail without these changes. (= ordered-map some-record-with-same-keys) returns true without these changes, for example, whereas Clojure's built-in maps are never = to any record, by design, since records are distinct types. (.equals ordered-map something-else) currently uses = to compare corresponding values in the maps, but other Clojure persistent maps use .equals to compare corresponding values. --- project.clj | 6 ++++-- src/flatland/ordered/map.clj | 9 +++++++- test/flatland/ordered/map_test.clj | 34 ++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/project.clj b/project.clj index 75fdbbe..31101cf 100644 --- a/project.clj +++ b/project.clj @@ -6,8 +6,10 @@ :dependencies [[org.clojure/clojure "1.6.0"] [org.flatland/useful "0.9.0"]] ;;:aliases {"testall" ["with-profile" "dev,default:dev,1.3,default:dev,1.4,default:dev,1.5,default:dev,1.7,default" "test"]} - :aliases {"testall" ["with-profile" "dev,default:dev,1.4,default:dev,1.5,default:dev,1.7,default" "test"]} - :profiles {:1.7 {:dependencies [[org.clojure/clojure "1.7.0"]]} + :aliases {"testall" ["with-profile" "dev,default:dev,1.4,default:dev,1.5,default:dev,1.7,default:dev,1.8,default:dev,1.9,default" "test"]} + :profiles {:1.9 {:dependencies [[org.clojure/clojure "1.9.0"]]} + :1.8 {:dependencies [[org.clojure/clojure "1.8.0"]]} + :1.7 {:dependencies [[org.clojure/clojure "1.7.0"]]} :1.6 {:dependencies [[org.clojure/clojure "1.6.0"]]} :1.5 {:dependencies [[org.clojure/clojure "1.5.1"]]} :1.4 {:dependencies [[org.clojure/clojure "1.4.0"]]} diff --git a/src/flatland/ordered/map.clj b/src/flatland/ordered/map.clj index e996f11..afac6fd 100644 --- a/src/flatland/ordered/map.clj +++ b/src/flatland/ordered/map.clj @@ -50,6 +50,7 @@ IPersistentMap (equiv [this other] (and (instance? Map other) + (or (not (map? other)) (instance? MapEquivalence other)) (= (.count this) (.size ^Map other)) (every? (fn [^MapEntry e] (let [k (.key e)] @@ -85,7 +86,13 @@ (toString [this] (str "{" (s/join ", " (for [[k v] this] (str k " " v))) "}")) (equals [this other] - (.equiv this other)) + (and (instance? Map other) + (= (.count this) (.size ^Map other)) + (every? (fn [^MapEntry e] + (let [k (.key e)] + (and (.containsKey ^Map other k) + (.equals (.val e) (.get ^Map other k))))) + (.seq this)))) (hashCode [this] (APersistentMap/mapHash this)) IHashEq diff --git a/test/flatland/ordered/map_test.clj b/test/flatland/ordered/map_test.clj index 8e690eb..2804e31 100644 --- a/test/flatland/ordered/map_test.clj +++ b/test/flatland/ordered/map_test.clj @@ -211,3 +211,37 @@ [[nil :a]] [[:a nil]] [[nil nil]])) + +(defrecord EmptyRec1 []) + +(defn is-same-collection [omap b] + (let [msg (format "(class omap)=%s (class b)=%s omap=%s b=%s" + (.getName (class omap)) (.getName (class b)) omap b) + b-persistent? (map? b) + should-be-=? (not (record? b))] + (is (= (count omap) (count b) (.size omap) (.size b)) msg) + (is (= (= omap b) should-be-=?) msg) + (is (= (= b omap) should-be-=?) msg) + (is (.equals ^Object omap b) msg) + (is (.equals ^Object b omap) msg) + (is (= (.hashCode ^Object omap) (.hashCode ^Object b)) msg) + ;; At least while CLJ-1372 is unresolved, Clojure persistent + ;; collections intentionally have different result for + ;; clojure.core/hash than otherwise = non-persistent collections. + (if (and b-persistent? (not (record? b))) + (is (= (hash omap) (hash b)) msg)))) + +(deftest map-collection-tests + (let [maps [ {} + (hash-map) + (array-map) + (sorted-map) + (sorted-map-by <) + (ordered-map) + (->EmptyRec1) + (java.util.HashMap.) ]] + (doseq [m maps] + (is-same-collection (ordered-map) m))) + + (is (= false (.equals {1 17N} (java.util.HashMap. {1 17})))) + (is (= false (.equals (ordered-map 1 17N) (java.util.HashMap. {1 17})))))