ViacomInc / data-point

JavaScript Utility for collecting, processing and transforming data
Apache License 2.0
67 stars 34 forks source link

Expose datapoint.resolve from Accumulator #338

Open acatl opened 5 years ago

acatl commented 5 years ago

Problem description:

This has come up over and over: expose the datapoint.resolve from the accumulator object, so a reducer function can internally have access to the same datapoint instance that generated it, this will allow creating more complex reducers.

on these discussions, @raingerber has raised the concern that reducers would 'hide' more transformations and would be more difficult to track down the path of the transformations being applied.

I agree with this argument, but also see the power on providing this functionality which to my opinion with the right coding, documentation and tracing tooling (an PR im working on) the concern should be taken care of to an extent.

Suggested solution:

Expose datapoint from inside the accumulator to be able to write reducer functions such as:

function foo(input, acc) {
   if(input == 'something') { 
      // this signature uses the same accumulator but overrides
      // its value with the second parameter passed to the function
      return acc.resolve(BazModelEntity, input)
   } else {
      // if the second parameter not passed, then it uses the
      // accumulator value as the input 
      return acc.resolve(BarModelEntity)
   }
   return foo
}
acatl commented 5 years ago

btw I have the code that does this almost ready to be sent as a PR, so issue is mainly to first ask for input on the idea

raingerber commented 5 years ago

Just to be clear, when you say "exposing datapoint," do you mean exposing a reduce function that is resolved with the underlying datapoint instance?

If so, why is it called reduce instead of resolve or transform (which are the method names that are used by the current API)? Also, if it's just exposing that function, would it provide access to other methods / properties of the datapoint instance (i.e. addEntities)?

acatl commented 5 years ago

it would be not exposing the instance but just the resolve method. the issue with that method is the conflict with the middleware we added resolve method, which prob we should change?

So if we do #206 we could use acc.resolve for this feature

acatl commented 4 years ago

206 was merged some time ago, we can now use .resolve, I will update the ticket