baconjs / bacon.matchers

Matchers API for Bacon.js
MIT License
21 stars 6 forks source link

Matching multiple clauses #10

Closed mkaemmerer closed 10 years ago

mkaemmerer commented 10 years ago

Looking to see if there's any interest in being able to match multiple clauses.

For example, maybe something like this:

  stream1.where()
    .any(
        m -> m.equalTo(Infinity),
        m -> m.equalTo(-Infinity)
    )

  stream2.is()
    .all(
        m -> m.containerOf('Inky'),
        m -> m.not().containerOf('Pinky'),
        m -> m.not().containerOf('Clyde')
    )

There almost certainly is a better way to write this, so I'm open to ideas. One problem with the above is that it is that all matchers evaluated against the same field key. It would be better to be able to run matchers against different fields, as in stream.filter(e -> e.x === 0 or e.y === 0).

jliuhtonen commented 10 years ago

In my opinion any (or some) and all (or every) sound like a nice idea. Some thoughts on options and how they could be implemented:

1) The trivial solution would be to just have an array of functions that could be applied to the current value and then the final boolean value could be reduced.

It would work like

stream1.where().any(
v -> v.indexOf('Inky') >= 0
v -> !v.indexOf('Pinky') >= 0
...
)

It would be really flexible and easy to implement, but you couldn't use any of those matchers bacon.matchers already has.

2) The option you suggested would be more... interesting implementation-wise. I suppose all of those m values to the functions are .is()-matcher objects that are generated for each value of the parent stream, right?

Then we would have to introduce something to do a flatMap to the source stream. Then there is still the matter of applying the operation (map/filter for is and where respectively). Luckily the methods are working so, that we could use the identity function (say _.id) with operation on flatMapped stream and everything would work as expected. Requires some refactoring though.

Then maybe something like this for any could work

context["any"] = flatMap2((val , arr) ->
      matchStreams = arr.map((f) -> f(Bacon.once(val).is()))
      Bacon.mergeAll(matchStreams).fold(false, (acc, curr) -> acc || curr)
    )

Probably there are more elegant ways of doing this, too.

But this again would constrain the usage to the matchers that have been implemented in bacon.matchers.

Not sure which one would be better. Or something else entirely? Any thoughts?

mkaemmerer commented 10 years ago

Now that you mention it, I think I would be in favor of using the names every/some instead of all/any. Might be less jarring to people who expect all to be a fold over collections of booleans.

Implementation-wise, I suspect the m objects would need to be a matcher object that worked point-wise (i.e. on regular, non-reactive values). We could accomplish this without too much refactoring by calling addMatchers on a vanilla object with suitable definitions for apply*. The implementation of all and any would handle running them on observables. This way we could avoid all the flatMap trickiness.

# 'm' is an object such that
m.greaterThan(3)
# is equivalent to
v -> v > 3

I like the extra generality that your definition has, in passing raw elements instead of matcher objects. But I also dislike the lack of clarity that accompanies it. In particular, I personally am unlikely to use that formulation of every since it doesn't seem to have a clear advantage over filter.

#Not bad...
stream1.where().every(
  v -> v.indexOf('Inky') >= 0
  v -> !v.indexOf('Pinky') >= 0
)

#...but this seems clearer and more concise to me.
stream1
  .filter(v -> v.indexOf('Inky') >= 0)
  .filter(v -> !v.indexOf('Pinky') >= 0)
jliuhtonen commented 10 years ago

Sounds like that would work. What I forgot to mention is, that in simple cases using .and() and .or() for properties is (if applicable) in my opinion simpler, although every/some could have their uses in cases where there are more clauses.

prop1.is().containerOf('Inky').and(prop1.is().not().containerOf('Pinky'))
mkaemmerer commented 10 years ago

Yeah, I like and/or for the boolean property case. every/some seems better suited to filtering than to mapping.

jliuhtonen commented 10 years ago

True, and memberOf covers things like stream.is().memberOf(['Inky', 'Pinky']).

The thing that I'm not sure about in the "custom" matchers objects given to the clause functions is that the signature is then Matcher -> Boolean. The matcher is kind of a special matcher that's methods do not produce an observable like usually. You also can't access the value either to do something different like use a function that you already have somewhere else.

What if the functions received observables, so the signature for those functions would be Observable[a] -> Observable[Boolean]. That would make it possible to use matchers or something else as well. Given this and if some/every would be only available for filtering it would look like:

stream2.every(
        s -> s.is().containerOf('Inky'),
        s -> s.map(isACoolName)
    )

So that could be for example something like

  Bacon.Observable::every = ((fs...) ->
    this.flatMap((value) ->
      valueStream = Bacon.once(value)
      matchStreams = fs.map((f) -> f(valueStream))
      isEvery = Bacon.mergeAll(matchStreams).fold(true, (acc, curr) -> acc && curr)
      valueStream.filter(isEvery)
    ))
mkaemmerer commented 10 years ago

The signature I had in mind is was actually "Matcher -> (Value -> Boolean)", which seems really odd to me now that I think about it. "Value -> Boolean" would be less strange, and you could still accomplish the readable syntax part by just creating your own matcher.

mkaemmerer commented 10 years ago

I think you hit the nail on the head with making it "Observable[Value] -> Observable[Boolean]". That covers both my need for nice syntax as well as my need for generality.

I'm pretty sure there's a much simpler and more efficient implementation of some/each. The way you have it, it seems to be re-evaluating the matcher functions every time the observable updates, which shouldn't be necessary. Here's my first shot at it:

Bacon.Observable::every = ((fs...) -> 
  matches = fs.map((f) -> f(this))
  isEvery = Bacon._.fold(matches, (x,y) -> x.and(y), Bacon.constant(true))
  this.filter(isEvery)
)

Bacon.Observable::some = ((fs...) ->
  matches = fs.map((f) -> f(this))
  isSome  = Bacon._.fold(matches, (x,y) -> x.or(y), Bacon.constant(false))
  this.filter(isSome)
)

I'll PR soon, once I have a chance to try it out and write some tests.

jliuhtonen commented 10 years ago

Yes, that is more simple. Great!