Clojure

'get' should throw exception on non-Associative argument

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test
  • Approval:
    Triaged

Description

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.

Activity

Stuart Sierra made changes -
Field Original Value New Value
Patch Code and Test [ 10002 ]
Description The implementation of clojure.core/get returns null if its argument is not a valid associative collection. However, calling 'get' on something which is neither nil nor an Associative collection is almost certainly a programmer error, and should be indicated by an exception.

CLJ-932 was accepted as a similar enhancement to 'clojure.core/contains?'
The implementation of clojure.core/get returns null if its argument is not a valid associative collection. However, calling 'get' on something which is neither nil nor an Associative collection is almost certainly a bug, and should be indicated by an exception.

This behavior can obscure common programmer errors such as:

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

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

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

Attached patch 0001 throws an IllegalArgumentException as the fall-through case of RT.getFrom.
Attachment 0001-CLJ-1107-Throw-exception-for-get-called-on-unsupport.patch [ 11672 ]
Hide
Andy Fingerhut added a comment -

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.

Show
Andy Fingerhut added a comment - 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.
Andy Fingerhut made changes -
Attachment clj-1107-throw-on-get-for-unsupported-types-patch-v2.txt [ 12004 ]
Alex Miller made changes -
Approval Triaged [ 10120 ]
Hide
Andy Fingerhut added a comment -

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.

Show
Andy Fingerhut added a comment - 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.
Hide
Stuart Sierra added a comment -

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

Show
Stuart Sierra added a comment - New patch 0003-CLJ-1107-Throw-exception-for-get-on-unsupported-type.patch created from master at 5cc167a.
Stuart Sierra made changes -
Description The implementation of clojure.core/get returns null if its argument is not a valid associative collection. However, calling 'get' on something which is neither nil nor an Associative collection is almost certainly a bug, and should be indicated by an exception.

This behavior can obscure common programmer errors such as:

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

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

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

Attached patch 0001 throws an IllegalArgumentException as the fall-through case of RT.getFrom.
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:

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

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

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.
Stuart Sierra made changes -
Alex Miller made changes -
Priority Minor [ 4 ] Major [ 3 ]
Hide
Andy Fingerhut added a comment -

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.

Show
Andy Fingerhut added a comment - 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.
Andy Fingerhut made changes -
Andy Fingerhut made changes -
Attachment clj-1107-throw-on-get-for-unsupported-types-patch-v2.txt [ 12004 ]

People

Vote (7)
Watch (4)

Dates

  • Created:
    Updated: