Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Functionally accumulating values #98

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ianbollinger
Copy link

One problem I've been having in writing PureScript bindings for rot.js is getting data out of the functions that rely on callbacks. Functions like FOV.compute require the user to pass in a callback that is obligated to accumulate mutable state. While this is possible in PureScript, it's inelegant and difficult to compose.

This pull request allows the user to instead accumulate a value in a functional manner. Functions like FOV.compute now take an additional argument, initialValue that is passed through to the
callback as the last argument, which then returns the accumulator which is again passed to the callback, and so on. Finally, FOV.compute returns the accumulator when finished. Because these
have been tacked on to the parameters list, the change should be backwards compatible. (undefined will be accumulated.)

Optimally, you'd be able to do this for the map and lighting interfaces as well. Unfortunately, they already return a value. The only value they return, however, is this. I'd personally change them to return an accumulator as well, but this might break existing code.

Anyway, I understand this is a weird request and will understand if you don't like it.

PS. Sorry about deleting the trailing whitespace. I keep forgetting to turn of that "feature" of Atom.

One problem I've been having in writing PureScript bindings for
rot.js is getting data out of the functions that rely on callbacks.
Functions like FOV.compute require the user to pass in a callback
that is obligated to accumulate mutable state. While this is possible
in PureScript, it's inelegant and difficult to compose.

This pull request allows the user to instead accumulate a value
in a functional manner. Functions like FOV.compute now take an
additional argument, `initialValue` that is passed through to the
callback as the last argument, which then returns the accumulator
which is again passed to the callback, and so on. Finally,
FOV.compute returns the accumulator when finished. Because these
have been tacked on to the parameters list, the change should be
backwards compatible. ("undefined" will be accumulated.)

Optimally, you'd be able to do this for the map and lighting
interfaces as well. Unfortunately, they already return a value.
The only value they return, however, is "this". I'd personally
change them to return an accumulator as well, but this might break
existing code.

Anyway, I understand this is a weird request and will understand if
you don't like it.
@ondras
Copy link
Owner

ondras commented Jul 20, 2016

Hi Ian,

(removing the trailing whitespace is very OK -- I like that Atom feature myself and let the editor eradicate it freely)

this is a tricky one indeed. I understand your motivation, but I see your proposed changes a little bit too invasive. In particular, they modify the relevant functions without actually improving them; they add complexity (one more argument, caring about returned value) but this complexity is not related to the underlying computation, but rather to the general code/value flow.

I still would like to satisfy your needs. What about some wrapper that takes care about the state management? This is a long blank shot, but I am thinking something similar to

function wrap(callbackBasedGenerator, thisp) {
  let results = [];
  let cb = function() { results.push(arguments); }
  return function() {
    callbackBasedGenerator.call(thisp, cb);
    return results;
  }
}

let pureGenerator = wrap(fov.compute, fov);
let collectedValues = pureGenerator(x, y, R);

I have exactly zero experience with PureScript, so I have no idea how feasible/possible would this approach be. Please let me know what you think...

@ianbollinger
Copy link
Author

ianbollinger commented Jul 20, 2016

Well, the problem with that approach (and what I'm currently using) is that it isn't data type agnostic. However, having to accumulate all the arguments in an array and then fold over that array isn't the end of the world. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants