cognitect-labs / transducers-js

Transducers for JavaScript
Apache License 2.0
1.6k stars 44 forks source link

Clarifying behavior of transducer/init #23

Closed tgriesser closed 9 years ago

tgriesser commented 9 years ago

Over on transduce, @kevinbeaty and I have been trying to determine the responsibility of @@transducer/init: discussion

The purpose of init as a place for defining the initial value of a reduction is clear, but the question is whether it should account for any state or context the object might already retain. In short, does init return:

  1. a new empty value:
return new this.constructor()
  1. the value:
return this
  1. a new value retaining some notion of state:
return new this.constructor(into({}, this.data)) 

It seems like the cleanest/most useful of those would be number 3, and applied to the example of Immutable.js List, it'd just be a matter of making the current value mutable/immutable:

List.prototype['@@transducer/init'] = function() {
  return this.asMutable();
};

List.prototype['@@transducer/step'] = function(result, arr) {
  return result.set(arr[0], arr[1]);
};

List.prototype['@@transducer/result'] = function(obj) {
  return obj.asImmutable();
};

Thoughts?

tgriesser commented 9 years ago

Actually, I think I just realized why you'd want to have init return an empty value, regardless it might be good to get the official word here and update the docs on what an expected value should be when it's not otherwise calling another xf[@@transducer/init']()

I guess also on that note, could the docs be amended to reflect that not all three are "required" to be considered a transformer - only step, if that is what was decided in #20.

swannodette commented 9 years ago

@tgriesser it is not the purpose of init to return any specific thing. It takes no arguments. It should only be invoked in the case that transduce is invoked with 3 instead of 4 arguments.

tgriesser commented 9 years ago

Great, thanks for the clarification.