Clojurians Log v2

Check the git history, but I suspect the answer is just “no one noticed”. In early days cljs was written quickly to achieve parity with clj and corners were cut

A quick 'git blame' on the source file containing defn of contains? in ClojureScript shows it was last changed in 2013, and that was a one-line change on the line that uses get in the implementation of contains?. Seeing what it was before that is taking a bit longer, because the file was moved from src/cljs/core.cljs to src/main/cljs/core.cljs in 2015, after that change was made, and I'm still trying to figure out how to see history of a file that has been removed...

Looks like contains? called -lookup before this commit https://github.com/clojure/clojurescript/commit/7e7f3a76eefd621165259c0f24d6507318dca029 and changed to use get with that commit, along with many other changes. Not clear to me why, but I've spent almost no time looking at ClojureScript internals, compared to Clojure/JVM internals.

From looking further at the changes to get in that commit, it appears that get was generalized to work on all things implementing ILookup, but also to work on Google Closure arrays and JavaScript strings. contains? changing to use get at the same time enabled contains? to work on those types, too.

gotcha, the issue with the get approach is that it forces the data structure to realize the value, which a specialized -contains-key? can do without realizing the value. I guess the most compelling argument to change this is the parity, given Clojure contains? does work with -contains-key?

this is what CLJ contains? is:

public static Object contains(Object coll, Object key) {
        if (coll == null) {
            return F;
        } else if (coll instanceof Associative) {
            return ((Associative)coll).containsKey(key) ? T : F;
        } else if (coll instanceof IPersistentSet) {
            return ((IPersistentSet)coll).contains(key) ? T : F;
        } else if (coll instanceof Map) {
            Map m = (Map)coll;
            return m.containsKey(key) ? T : F;
        } else if (coll instanceof Set) {
            Set s = (Set)coll;
            return s.contains(key) ? T : F;
        } else if (!(key instanceof Number) || !(coll instanceof String) && !coll.getClass().isArray()) {
            if (coll instanceof ITransientSet) {
                return ((ITransientSet)coll).contains(key) ? T : F;
            } else if (coll instanceof ITransientAssociative2) {
                return ((ITransientAssociative2)coll).containsKey(key) ? T : F;
            } else {
                throw new IllegalArgumentException("contains? not supported on type: " + coll.getClass().getName());
            }
        } else {
            int n = ((Number)key).intValue();
            return n >= 0 && n < count(coll);
        }
    }

so CLJS version could check for the protocols as clojure does, before going down to get

I do not understand what you mean when you say "Clojure does work with -contains-key?". There is no -contains-key? in Clojure.

@andy.fingerhut the equivalent is the containsKey method

well, Associative.containsKey and IPersistentSet.contains()

@wilkerlucio I wouldn’t look for a deep reason here; I really think it’s either oversight or expedience

there were similar issues with find which I had no trouble getting a patch accepted for: https://clojure.atlassian.net/browse/CLJS-2136

cool, thanks, I'll open a ticket

https://clojure.atlassian.net/browse/CLJS-3283