kefirjs / kefir

A Reactive Programming library for JavaScript
https://kefirjs.github.io/kefir/
MIT License
1.87k stars 97 forks source link

Passing index to mapping functions #46

Closed gnapse closed 9 years ago

gnapse commented 9 years ago

I'm somewhat new to FRP, and I only have experience with ReactiveX (specifically RxJS). I'm looking to make the move to Kefir. The main advantage over RxJS is that Kefir allows to convert error events to values, and that error events do not halt the stream.

However, when porting some of my code to Kefir, I realized that the functions passed to map, flatMap, etc. do not get the index alongside the value. In RxJS, you can do the following:

observable.flatMap(function(value, index) {
  // index is now 0, 1, etc.
});

Any reason for this design decision? Could it be added? Or is there any known workaround that you could suggest? The only "solution" I see is to keep an index variable from the closure around that call, and increment it myself:

var index = 0;
observable.flatMap(function(value) {
  // do your stuff
  index++;
  // return result
});

But besides the lack of elegance, I'm worried about any potential issues with thread safety.

rpominov commented 9 years ago

There is number of reasons why we don't pass index in .map, .filter etc.:

  1. It might break composability, the classic example is ['1','2','3'].map(parseInt) // => [1, NaN, NaN].
  2. It might cause minor performance penalty.
  3. It'll make .map, .filter etc. stateful, and we have minor issue with stateful observables. So stateless always better than satetful for us.

I think solution with index variable is ok, but if you use that feature a lot you can define a helper method like this:

Kefir.Observable.prototype.mapWithIndex = function(fn) {
  var i = 0;
  return this.map(function(x) {
    return fn(x, i++);
  });
};

// Usage
stream2 = stream1.mapWithIndex(function(x, i) {...});
gnapse commented 9 years ago

The proposed helper method looks good to me. I still think it should be provided by Kefir, specially like this, with a different method (it solves point number 1 above regarding composability). You can always mention any possible caveats in the documentation. But that's just me. I can perfectly live with the custom helper. And now is here in the issues for anyone else to discover. Thanks!