Clojure

Do not use hash-map constructor in destructuring to avoid multiple key check.

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Trivial Trivial
  • Resolution: Completed
  • Affects Version/s: Release 1.2
  • Fix Version/s: Release 1.5
  • Component/s: None
  • Labels:
    None
  • Environment:
    Clojure versions 1.2 and above
  • Patch:
    Code and Test
  • Approval:
    Ok

Description

What I did:

(defn number-to-string
  [& {fmt :format locale :locale :or {locale (Locale/getDefault)}}]
  (fn [v]
    (String/format locale fmt (to-array [v]))))

(defn double-to-string
  [& options]
  (apply number-to-string :format "%f" options))

(def us-number (double-to-string :locale Locale/US))
(def legacy-number (double-to-string :format "% 3.2f"))

(us-number 3.14159)
(legacy-number 3.14159)

What I expected:

"3.14159"
"  3,14"

What I got:

java.lang.IllegalArgumentException: Duplicate key: :format

Using into or a combination of reduce and assoc instead of hash-map would allow this without breaking things.

If this is a desired modification, I can provide a patch. (CA is filed.)

Activity

Hide
Meikel Brandmeyer added a comment -

Build map in destructuring step by step instead of calling hash-map.

Show
Meikel Brandmeyer added a comment - Build map in destructuring step by step instead of calling hash-map.
Hide
Andy Fingerhut added a comment -

clj-763-destructuring-allow-dup-keys-patch2.txt dated Mar 26, 2012 applies cleanly to latest master and passes all tests, including a new one added based upon Meikel's report that fails without his fix. Supersedes his earlier patch 0001-Avoid-duplicate-key-check-in-destructuring.patch of Mar 21, 2011. Meikel and I have both signed CAs.

Show
Andy Fingerhut added a comment - clj-763-destructuring-allow-dup-keys-patch2.txt dated Mar 26, 2012 applies cleanly to latest master and passes all tests, including a new one added based upon Meikel's report that fails without his fix. Supersedes his earlier patch 0001-Avoid-duplicate-key-check-in-destructuring.patch of Mar 21, 2011. Meikel and I have both signed CAs.
Hide
Rich Hickey added a comment -

I'm happy to see this improved, but it has to be fast. into + partition is not fast enough down here. Probably should instead call non-checking map ctor

Show
Rich Hickey added a comment - I'm happy to see this improved, but it has to be fast. into + partition is not fast enough down here. Probably should instead call non-checking map ctor
Hide
Meikel Brandmeyer added a comment -

Map destructuring without duplicate check with test case

Show
Meikel Brandmeyer added a comment - Map destructuring without duplicate check with test case
Hide
Meikel Brandmeyer added a comment - - edited

Uploaded 0001-Do-not-check-for-duplicates-in-destructuring-map-cre.patch with a faster implementation accessing the unchecked constructor directly plus a simplified test case. Supersedes the two previous patches. (I can delete only mine, but not Andy's.)

Show
Meikel Brandmeyer added a comment - - edited Uploaded 0001-Do-not-check-for-duplicates-in-destructuring-map-cre.patch with a faster implementation accessing the unchecked constructor directly plus a simplified test case. Supersedes the two previous patches. (I can delete only mine, but not Andy's.)
Hide
Andy Fingerhut added a comment -

Removing patch clj-763-destructuring-allow-dup-keys-patch2.txt dated Mar 26 2012 in favor of Meikel's Apr 23 2012 patch, which fixes the problem in a faster way, and its test is more straightforward.

Show
Andy Fingerhut added a comment - Removing patch clj-763-destructuring-allow-dup-keys-patch2.txt dated Mar 26 2012 in favor of Meikel's Apr 23 2012 patch, which fixes the problem in a faster way, and its test is more straightforward.
Hide
Meikel Brandmeyer added a comment -

Deleted obsolete patch.

Show
Meikel Brandmeyer added a comment - Deleted obsolete patch.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: