'get' should throw exception on non-Associative argument

Question

The implementation of {{clojure.core/get}} returns {{nil}} if its argument is not an associative collection.

This behavior can obscure common programmer errors such as:


(def a (atom {:a 1 :b 2})

(:foo a)   ; forgot to deref a
;;=> nil


Calling {{get}} on something which is neither {{nil}} nor an Associative collection is almost certainly a bug, and should be indicated by an exception.

CLJ-932 was accepted as a similar enhancement to {{clojure.core/contains?}}

*Patch:* 0003-CLJ-1107-Throw-exception-for-get-on-unsupported-type.patch

*Approach:* Throw IllegalArgumentException as final fall-through case in RT.getFrom instead of returning nil.

*Also see:* CLJ-969

Answer 1

Comment made by: richhickey

This would be a breaking change

Answer 2

even if the initial design of clojure.core/get was confusing//wrong, now it is too late to change. This change do not worth. Will break too many programs/libraries.
the better that we can do now is create a clojure.core/get2 or my-lib.core/get with this new behavior.

Also, I can't say that this get2 has a better/righter behavior than our get. It is just another set of rules for get. As you propose this new set of rules, what guarantee that in another day, someone else will not appear and propose a new set of rules for get? maybe a get3? how do you know when to stop?

Answer 3

Comment made by: jafingerhut

Patch clj-1107-throw-on-get-for-unsupported-types-patch-v2.txt dated May 24 2013 is identical to 0001-CLJ-1107-Throw-exception-for-get-called-on-unsupport.patch dated Nov 13 2012, except it applies cleanly to latest master. A recent commit for CLJ-1099 changed many IllegalArgumentException occurrences to Throwable in the tests, which is the only thing changed in this updated patch.

Answer 4

Comment made by: jafingerhut

Patch clj-1107-throw-on-get-for-unsupported-types-patch-v2.txt applied cleanly to latest Clojure master as of Jan 23 2014, but no longer does with commits made to Clojure between then and Jan 30 2014. I have not checked to see how difficult or easy it may be to update this patch.

Answer 5

Comment made by: stuart.sierra

New patch 0003-CLJ-1107-Throw-exception-for-get-on-unsupported-type.patch created from master at 5cc167a.

Answer 6

Comment made by: jafingerhut

Patch clj-1107-throw-on-unsupported-get-v4.patch dated Mar 26 2014 is identical to Stuart Sierra's patch 0003-CLJ-1107-Throw-exception-for-get-on-unsupported-type.patch, and retains his authorship. The only difference is in one line of diff context required in order to make it apply cleanly to latest master.

Answer 7

_Comment made by: stuart.sierra_

Arguably so was CLJ-932 ({{contains?}}), which did "break" some things that were already broken.

This is a more invasive change than CLJ-932, but I believe it is more likely to expose hidden bugs than to break intentional behavior.

Answer 8

_Comment made by: alex+import_

Is it more idiomatic to use "({:a 1}, :a)" and a safe replacement to boot? E.g. could you mass replace "(get " with "(" in a code base, in order to find bugs? I am still learning the language, and not young anymore, and couldn't reliably remember the argument order. So, I found it easier to avoid (get) with maps anyways. Without it I can put the map first or second.

Answer 9

Comment made by: alexmiller

Presumably this could also now be accomplished via a spec on "get".

Answer 10

Reference: https://clojure.atlassian.net/browse/CLJ-1107 (reported by stuart.sierra)

Answer 11

I was under the assumption that (get m k) is equivalent to (val (find m k)), but that is not the case when m is not associative, which is very suprising to me.

find throws an exception when m is not associative, but get just returns nil.

I think get should also throw when the first argument is not associative.
Returning nil on non-associative args will only obscure bugs, I can't think of any valid use case.