Clojure

Minor Code Cleanup in core.reducers: use required walk, drop this for coll

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Trivial Trivial
  • Resolution: Unresolved
  • Affects Version/s: Release 1.5
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code

Description

First, core.reducers requires clojure.walk :as walk, but does not use the alias.
Second, the two arity implementation of coll-reduce in function reducer uses 'this', whereas similar implementations in that file use 'coll'. AFAICT it makes no difference to use 'coll' (all tests pass, no change in performance) and it is more in line with the rest of the code.

The two things seem small enough to be put into one cleanup case.

Activity

Hide
Jozef Wagner added a comment -

your patch is wrong. If you want to replace this with coll, you have to allso call xf on f1.

(clojure.core.protocols/coll-reduce this f1 (f1))

becomes

(clojure.core.protocols/coll-reduce coll (xf f1) (f1))
Show
Jozef Wagner added a comment - your patch is wrong. If you want to replace this with coll, you have to allso call xf on f1.
(clojure.core.protocols/coll-reduce this f1 (f1))
becomes
(clojure.core.protocols/coll-reduce coll (xf f1) (f1))
Hide
Stefan Kamphausen added a comment -

Scary, that the test suite did not detect that.

Show
Stefan Kamphausen added a comment - Scary, that the test suite did not detect that.
Hide
Andy Fingerhut added a comment -

Is it straightforward to add a test that would have detected that?

Show
Andy Fingerhut added a comment - Is it straightforward to add a test that would have detected that?
Hide
Stefan Kamphausen added a comment -

I will happily look into that. It may take a few days, before I'll find the time, though.

Show
Stefan Kamphausen added a comment - I will happily look into that. It may take a few days, before I'll find the time, though.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated: