ClojureScript

clojure.core.reducers/fold broken

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test

Description

clojure.core.reducers/fold currently does not work correctly if both a reducef and combinef are passed as arguments.

It is expected that (r/fold + + [1 2 3]) would return 6 (as do (r/fold + [1 2 3]) and (r/reduce + [1 2 3])). However, this is not the case because reducers.cljs currently defines fold as follows:

(def fold reduce)

As a result, the second + is used as the initial value for the reduce operation (instead of the value returned by ).

The attached patch fixes this bug. It also adds support for the protocol CollFold in CLJS.

(I will send in the CA tomorrow.)

  1. cljs_700.patch
    29/Nov/13 3:58 PM
    5 kB
    Jonas De Vuyst
  2. cljs-fold-fix.patch
    27/Nov/13 12:53 AM
    4 kB
    Jonas De Vuyst
  3. cljs-fold-fix-2.patch
    29/Nov/13 1:45 PM
    4 kB
    Jonas De Vuyst

Activity

Hide
David Nolen added a comment -

This mostly looks good but clojure.lang.PersistentHashMap should be cljs.core/PersistentHashMap and we should have a corresponding test.

Show
David Nolen added a comment - This mostly looks good but clojure.lang.PersistentHashMap should be cljs.core/PersistentHashMap and we should have a corresponding test.
Hide
Jonas De Vuyst added a comment - - edited

I added a new patch, in which cljs.core/PersistentHashMap is substituted for clojure.lang.PersistentHashMap – although that part is actually commented out because CLJS maps do not have a method .fold.

I also added tests for reducing and folding of maps (which I also tested in Clojure).

Show
Jonas De Vuyst added a comment - - edited I added a new patch, in which cljs.core/PersistentHashMap is substituted for clojure.lang.PersistentHashMap – although that part is actually commented out because CLJS maps do not have a method .fold. I also added tests for reducing and folding of maps (which I also tested in Clojure).
Jonas De Vuyst made changes -
Field Original Value New Value
Attachment cljs-fold-fix-2.patch [ 12498 ]
Hide
David Nolen added a comment -

The patch has not been formatted so that it can be applied with git am. Please generate the patch following these instructions - http://github.com/clojure/clojurescript/wiki/Patches.

Show
David Nolen added a comment - The patch has not been formatted so that it can be applied with git am. Please generate the patch following these instructions - http://github.com/clojure/clojurescript/wiki/Patches.
Hide
Jonas De Vuyst added a comment -

Oh, sorry. I have attached a new patch.

Show
Jonas De Vuyst added a comment - Oh, sorry. I have attached a new patch.
Jonas De Vuyst made changes -
Attachment cljs_700.patch [ 12499 ]
David Nolen made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
David Nolen made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: