Clojure

assoc should throw exception if missing val for last key

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: Release 1.2, Release 1.3, Release 1.4
  • Fix Version/s: Release 1.5
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test
  • Approval:
    Ok

Description

https://groups.google.com/forum/?fromgroups=#!topic/clojure/k2R4OdPUCzg

Suggested by Ambrose Bonnaire-Sergeant:

I think assoc should throw an error when applied with uneven arguments.

Currently, the "missing" value is just replaced with nil.

(assoc {} :a 1 :b)
;=> {:a 1, :b nil}

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Andy Fingerhut added a comment -

Patch clj-1052-assoc-should-throw-exc-if-missing-val-patch1.txt dated Aug 29 2012 makes assoc throw an IllegalArgumentException if more than one key is given, but the last key has no value. It includes a few simple test cases with correct and incorrect arguments to assoc.

Show
Andy Fingerhut added a comment - Patch clj-1052-assoc-should-throw-exc-if-missing-val-patch1.txt dated Aug 29 2012 makes assoc throw an IllegalArgumentException if more than one key is given, but the last key has no value. It includes a few simple test cases with correct and incorrect arguments to assoc.
Hide
Permalink
Ambrose Bonnaire-Sergeant added a comment -

IMO if the error message included something like "assoc expects even number of arguments after target, found odd number", or some mention of the number of arguments the error would be clearer to me.

Show
Ambrose Bonnaire-Sergeant added a comment - IMO if the error message included something like "assoc expects even number of arguments after target, found odd number", or some mention of the number of arguments the error would be clearer to me.
Hide
Permalink
Andy Fingerhut added a comment -

clj-1052-assoc-should-throw-exc-if-missing-val-patch2.txt is identical to the now-removed -patch1.txt, except for the text of the exception thrown, updated as per Ambrose's suggestion.

Show
Andy Fingerhut added a comment - clj-1052-assoc-should-throw-exc-if-missing-val-patch2.txt is identical to the now-removed -patch1.txt, except for the text of the exception thrown, updated as per Ambrose's suggestion.
Hide
Permalink
Stuart Halloway added a comment -

The patch appears correct. It does introduce a single extra (next) per iteration into the success path, although that seems unlikely to dominate the work. Wouldn't hurt to add as assessment showing this is no slower for correct programs.

Show
Stuart Halloway added a comment - The patch appears correct. It does introduce a single extra (next) per iteration into the success path, although that seems unlikely to dominate the work. Wouldn't hurt to add as assessment showing this is no slower for correct programs.
Hide
Permalink
Andy Fingerhut added a comment - - edited

Test platform: Mac OS X 10.6.8 + Oracle/Apple Java 1.6.0_35 Java HotSpot(TM) 64-Bit Server VM

With latest Clojure master as of Sep 19 2012:

Clojure 1.5.0-master-SNAPSHOT
user=> (set! unchecked-math true)
true
(defn count-maps [n]
(let [base {:a 1}]
(loop [i n
sum 0]
(if (zero? i)
sum
(let [m1 (assoc base :b 2 :c 3 :d 4 :e 5 :f 6 :g 7 :h 8 :i 9 :j 10)]
(recur (dec i) (+ sum (count m1))))))))
#'user/count-maps
user=> (time (count-maps 10000000))
"Elapsed time: 48784.077 msecs"
100000000
user=> (time (count-maps 10000000))
"Elapsed time: 49028.52 msecs"
100000000
user=> (time (count-maps 10000000))
"Elapsed time: 50314.729 msecs"
100000000

Same Clojure version, plus the patch that was screened:

user=> (time (count-maps 10000000))
"Elapsed time: 49576.191 msecs"
100000000
user=> (time (count-maps 10000000))
"Elapsed time: 49957.866 msecs"
100000000
user=> (time (count-maps 10000000))
"Elapsed time: 52149.998 msecs"
100000000

(average of 3 times after patch) / (average of 3 times before patch) = 1.0240

So 2.4% slowdown on average for that test case. I should add that I'm not a statistician, but note that this 2.4% difference is less than the variation in run time from one run to the next of the same experiment. Likely any real statistician would recommend collecting a lot more data before asserting there is a change in performance.

Show
Andy Fingerhut added a comment - - edited Test platform: Mac OS X 10.6.8 + Oracle/Apple Java 1.6.0_35 Java HotSpot(TM) 64-Bit Server VM With latest Clojure master as of Sep 19 2012: Clojure 1.5.0-master-SNAPSHOT user=> (set! unchecked-math true) true (defn count-maps [n] (let [base {:a 1}] (loop [i n sum 0] (if (zero? i) sum (let [m1 (assoc base :b 2 :c 3 :d 4 :e 5 :f 6 :g 7 :h 8 :i 9 :j 10)] (recur (dec i) (+ sum (count m1)))))))) #'user/count-maps user=> (time (count-maps 10000000)) "Elapsed time: 48784.077 msecs" 100000000 user=> (time (count-maps 10000000)) "Elapsed time: 49028.52 msecs" 100000000 user=> (time (count-maps 10000000)) "Elapsed time: 50314.729 msecs" 100000000 Same Clojure version, plus the patch that was screened: user=> (time (count-maps 10000000)) "Elapsed time: 49576.191 msecs" 100000000 user=> (time (count-maps 10000000)) "Elapsed time: 49957.866 msecs" 100000000 user=> (time (count-maps 10000000)) "Elapsed time: 52149.998 msecs" 100000000 (average of 3 times after patch) / (average of 3 times before patch) = 1.0240 So 2.4% slowdown on average for that test case. I should add that I'm not a statistician, but note that this 2.4% difference is less than the variation in run time from one run to the next of the same experiment. Likely any real statistician would recommend collecting a lot more data before asserting there is a change in performance.
Hide
Permalink
Andy Fingerhut added a comment -

clj-1052-assoc-should-throw-exc-if-missing-val-patch3.txt dated Oct 5 2012 is the same as the previous patch clj-1052-assoc-should-throw-exc-if-missing-val-patch2.txt dated Aug 29 2012 except some context lines have been updated so that it applies cleanly using git. The older version will be removed in a minute.

Show
Andy Fingerhut added a comment - clj-1052-assoc-should-throw-exc-if-missing-val-patch3.txt dated Oct 5 2012 is the same as the previous patch clj-1052-assoc-should-throw-exc-if-missing-val-patch2.txt dated Aug 29 2012 except some context lines have been updated so that it applies cleanly using git. The older version will be removed in a minute.

People

Vote (8)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.