<< Back to previous view

[CCACHE-29] Is IPersistentCollection definition of cons correct? Created: 18/Nov/12  Updated: 04/Aug/14  Resolved: 04/Aug/14

Status: Closed
Project: core.cache
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Brian Marick Assignee: Fogus
Resolution: Completed Votes: 0
Labels: None


Here's the definition of `cons` for caches:

(cons _# elem#
(clojure.core/cons ~base-field elem#))

This seems wrong to me. Note first that the arguments to `clojure.core/cons` are in the wrong order. As a result, the result of (for example) conj is incorrect. Consider this:

user=> (def starts-with-a (cache/fifo-cache-factory {:a 1} :threshold 3))

user=> starts-with-a
{:a 1}
user=> (conj starts-with-a [:c 3])
({:a 1} :c 3)

Even if the argument order was correct, the result would still be a sequence rather than the type of the base field. I think you want something more like

(cons this# elem#
(apply assoc this# elem#))

After all, this particular collection is an IPersistentMap, so its `conj` and `into` behavior should be the same as other objects for which `map?` is true.

Comment by Ambrose Bonnaire-Sergeant [ 04/Aug/14 10:16 PM ]

Fixed https://github.com/clojure/core.cache/commit/19574f5da66b45f5fec0a9e2980559c52152f85e

Generated at Thu Mar 23 01:30:02 CDT 2017 using JIRA 4.4#649-r158309.