Clojurians Log v2

I made a custom Clojure type! https://github.com/martinklepsch/custom-comparator-set First time I’ve done this. It’s pretty little code easy to review I guess. Comments on style/indentation/thinking/everything most welcome :simple_smile:

Here’s v2: https://github.com/martinklepsch/custom-comparator-set :simple_smile:

@martinklepsch: do you need to define .equals and .hashcode?

@xeqi: it probably would make sense :simple_smile:

I’m just doing that for the cljs side of things

I imagine a test case would be trying to use the sets as keys in a {}

also, tried to build locally to test and got "Could not find artifact adzerk:boot-test:jar:1.0.5-SNAPSHOT"

Yeah, that’s a local mod for cljc testing. PR outstanding in the boot-test repo.

Sorry about that

Thanks for the test case idea, was actually wondering how I can test this :simple_smile:

how’s your clojure/java post coming along btw :simple_smile:

I'm not 100% on that being a good testcase fwiw

haha, it will have to be several. That much stuff could easily be an ebook

martinklepsch: for the print-method do doseq on the vals with print-method recursively

@xeqi: is this what you had in mind kind of?

(deftest hashing
  (let [cset (ccset/custom-comparator-set :k {:k "a"})
        kmap (-> {} (assoc cset 1) (assoc cset 2))]
    (t/is (= 2 (get kmap cset)))
    (t/is (= 1 (count kmap)))))

ignore this

(deftest hashing
  (let [cset1 (ccset/custom-comparator-set :k {:k "a"})
        cset2 (ccset/custom-comparator-set :k {:k "a"})
        kmap (-> {} (assoc cset1 1) (assoc cset2 2))]
    (t/is (= 2 (get kmap cset1)))
    (t/is (= 2 (get kmap cset2)))
    (t/is (= 1 (count kmap)))))

@ghadi: thanks, do you mean like this?

(defmethod print-method CustomComparatorSet [v ^java.io.Writer w]
  (.write w "#CustomComparatorSet{")
  (doseq [x (seq v)]
    (print-method x w)
    (.write w " "))
  (.write w "}"))

would I use interpose to add spaces between items?

no

this is good

you're doing imperative printing

keep it clear

@ghadi: if I want to elide the trailing space before the closing } I’d use loop and check for rest?

@martinklepsch yeah, something like that to see if you needed to override those methods. Did the test fail without custom .hashcode/.equals?

@xeqi: yes they’re failing

@xeqi: is there some place where I can lookup protocols/interfaces for clojure types?

like how do I figure out what Interface I have to implement for .hashcode and .equals?

@martinklepsch https://github.com/ztellman/collection-check is where I would start

Those two are on Object

@xeqi: where you would start with testing?

@martinklepsch I meant for finding protocols/interfaces for Clojure types. For finding out what classes/interfaces those methods belog to higher up, I don't know a good place

ok. thanks! :simple_smile:

in the process of adding collection-check to the tests

Though if I was testing your set, i'd highly consider throwing collection-check at (comparable-set identity) compared to #{}

that’s exactly what I’m doing :simple_smile:

@xeqi: do you think comparable-set is a better name (just asking because you used it)

Though that might not be quite right, cause is (= (c-s identity) (c-s #(%1))? How does the comparable effect equality? Etc etc

I hadn't though about it, just typed that cause I'm on mobile atm and didn't want to go back to the code to find real name

I don't have a good term for this concept

@martinklepsch back to actual review, could you delegate .contains to (contains? data (comparator v))? I think that would avoid building a set and walking every element each time.

@xeqi: good suggestion thanks!