Make assoc-some behaviour for nil map consistent with assoc

[dmcgillen]

Fixes https://github.com/weavejester/medley/issues/73

This makes the behaviour as it was in 1.5.0, which is more consistent with Clojure core fns.

Example	1.6.0	This PR
(medley/assoc-some nil :a 1)	{:a 1}	{:a 1}
(medley/assoc-some nil :a 1 :b 2)	java.lang.NullPointerException	{:a 1 :b 2}
(medley/assoc-some nil :a nil)	nil	nil
(medley/assoc-some nil :a nil :b nil)	java.lang.NullPointerException	nil

I think nil should be returned for a nil input when nothing is assoced. This seems consistent with things such as:

(dissoc {} :a) ;; => {}
(dissoc nil :a) ;; => nil

I ran it through some benchmarks given the recent changes were to improve performance to ensure it wasn't impacted. There is essentially no difference when running the benchmark provided in https://github.com/weavejester/medley/pull/68 (which you'd expect given it spends most of its time in the loop and the only change is an extra step at each of the beginning and the end):

(def alphabet '[a b c d e f g h i j k l m n o p q r s t u v w x y z])
(def test-map (zipmap alphabet (range)))
(def test-vec (interleave (shuffle alphabet) (repeatedly #(rand-nth [true nil]))))

(c/bench (apply medley/assoc-some test-map test-vec))

1.6.0:

"Evaluation count : 31912920 in 60 samples of 531882 calls.
             Execution time mean : 1.895565 µs
    Execution time std-deviation : 9.149761 ns
   Execution time lower quantile : 1.878465 µs ( 2.5%)
   Execution time upper quantile : 1.912932 µs (97.5%)
                   Overhead used : 1.933850 ns

Found 1 outliers in 60 samples (1.6667 %)
    low-severe   1 (1.6667 %)
 Variance from outliers : 1.6389 % Variance is slightly inflated by outliers

This PR:

             Execution time mean : 1.869713 µs
    Execution time std-deviation : 8.932134 ns
   Execution time lower quantile : 1.858134 µs ( 2.5%)
   Execution time upper quantile : 1.888022 µs (97.5%)
                   Overhead used : 1.933850 ns

A more realistic test I suppose is a more minimal assoc so that the change has more impact:

(c/bench (medley/assoc-some {} :a 1 :b 2))

1.6.0:

Evaluation count : 318338040 in 60 samples of 5305634 calls.
             Execution time mean : 188.908034 ns
    Execution time std-deviation : 16.461938 ns
   Execution time lower quantile : 163.524863 ns ( 2.5%)
   Execution time upper quantile : 218.983431 ns (97.5%)
                   Overhead used : 1.933850 ns

Found 2 outliers in 60 samples (3.3333 %)
    low-severe   1 (1.6667 %)
    low-mild     1 (1.6667 %)
 Variance from outliers : 63.5511 % Variance is severely inflated by outliers

This PR:

Evaluation count : 288534420 in 60 samples of 4808907 calls.
             Execution time mean : 205.147104 ns
    Execution time std-deviation : 4.090168 ns
   Execution time lower quantile : 196.734137 ns ( 2.5%)
   Execution time upper quantile : 211.873830 ns (97.5%)
                   Overhead used : 1.933850 ns

[tomdl89]

Good catch! It's a bit pedantic, but maybe it's best only to make acc persistent if it's not empty:

(defn assoc-some
  "Associates a key k, with a value v in a map m, if and only if v is not nil."
  ([m k v]
   (if (nil? v) m (assoc m k v)))
  ([m k v & kvs]
   (loop [acc (assoc-some-transient! (transient (or m {})) k v)
          kvs kvs]
     (cond
       (next kvs) (recur (assoc-some-transient! acc (first kvs) (second kvs))
                         (nnext kvs))
       (zero? (count acc)) m ; see CLJ-1872
       :else (persistent! acc)))))

[dmcgillen]

That looks a lot tidier to me

[tomdl89]

I only know this because I've been through the PR process here before, but I expect you'll be asked to keep your line widths ≤80 chars (that's why I put (nnext kvs)) on a newline).

[dmcgillen]

Thanks @tomdl89. I think I spent more time thinking about the formatting when I was looking at this earlier than the change itself! I've gone with what you've put above.

[weavejester]

Thanks! Can you change the commit message to:

Make assoc-some for nil map consistent with assoc

Fixes #73.

That will put the subject line of the commit message under 50 characters (see CONTRIBUTING.md), and link the commit back to the issue it solves.

[dmcgillen]

Done