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

be a well-behaved pull-stream #1

Closed
dominictarr opened this issue Jul 1, 2016 · 13 comments · Fixed by #2
Closed

be a well-behaved pull-stream #1

dominictarr opened this issue Jul 1, 2016 · 13 comments · Fixed by #2

Comments

@dominictarr
Copy link

If the sink always reads synchronously, this module should work fine,
however in some situations it will behave in correctly.

https://github.com/davidchase/pull-dom-events/blob/master/index.js#L7

If an event happens more than once before the sink has read again, it will call the callback twice.
(but with a new value)

There are different ways you could address this, depending on what behavior you want with dom events, but the simplest thing would be to unset callback before calling it.

if(callback) {
  let _cb = callback
  callback = null
  _cb(err, data)
}

then you know callback can never be called twice, even if it's called recursively (before itself has returned)

Another question is what do do about events that happen inbetween calls?
one idea would be to let them drop, so you could have a stream of mousemove events,
and whenever you call read you get either the most recent or the next mouse event.
If you just implement the above suggestion, you'll always get the next mouse event.

you could also keep the most recent event, and respond with that if it hasn't been read yet.

for keyboard events you might want to keep the sequence of events?

@davidchase
Copy link
Owner

@dominictarr thanks for creating this issue

would you say a step in the right direction would be to implement your above suggestion?

rather still new to the pull-stream way of things.. looking for some best practices

you could also keep the most recent event,

would something like a simple array buffer help in that situation ?

@dominictarr
Copy link
Author

yup. a stream of all the events would be correct. But might be unnecessary in some cases.

This is currently the best document about exactly what is pull-stream behavior.
https://github.com/pull-stream/pull-stream/blob/master/docs/spec.md

We are working on smoothing the learning curve, so feedback, questions, etc are appreciated!

davidchase pushed a commit that referenced this issue Jul 3, 2016
@davidchase
Copy link
Owner

i added a pr #2 not sure if that helps address the callback issue where it gets called twice...

@davidchase
Copy link
Owner

davidchase commented Jul 4, 2016

@dominictarr after reading a few times over the above spec link you shared i have a few questions:

does the person implementing a source have to pass the data to the callback from read in a different callstack iow using some kind of scheduler like a simple nextTick?

or what guarantees the asynchronicity of the source function?

Edit is there a better place to ask these questions?

@dominictarr
Copy link
Author

you can call back async. but I'd recommend calling back sync if you got your data sync. that is what most i've done in all my pull-streams. It makes it a little bit more tricky, but it also makes it easier to test, because sync enables you to reason more precisely about what order events happen in. Also, nextTick actually adds quite a bit of overhead. pull-streams are often long chains of simple transforms, that they can be async but do not need to be is very valuable.

@davidchase
Copy link
Owner

davidchase commented Jul 4, 2016

@dominictarr that makes sense, the reason i asked was this quote from the spec

A Source Stream (aka readable stream) is a asynchronous function

and when i was browsing the sources files i didnt see anything made me think it was async

also any thoughts on #2 ?

thanks for all of the help 😄

@dominictarr
Copy link
Author

by async function I mean just that it takes a callback function as the last argument, (in this case, always the second 2nd) which it eventually calls exactly once.

Some people say that you should always call a function either always sync or always async, but I disagree here. pull-stream work well with functions that may either be async or sync.

@davidchase
Copy link
Owner

Some people say that you should always call a function either always sync or always async

Isnt that because of zalgo? ...

also callback doesnt seem to always mean async ie: [1,2,3].forEach(callback) happens synchronously

@dominictarr
Copy link
Author

yes. it has been called "zalgo".

there are no formal definitions, but here are the distinctions that I make about uses of functions in node.

  • emitter.on('event', listener). listeners are functions that will be called zero or more times.
  • fs.readFile(filename, callback). callbacks are functions that are called exactly once (i.e. never twice, never not called) callbacks always have error first cb(err, data...)
  • array.forEach(lamba) otherwise, lamba (or just "function" or "iterator" or "mapper") are functions that are called zero or more times, but always called sync (i.e. never called again after the function they where passed to has returned)

Always making "async functions" callback after a time is pretty good advice, but never say "never", never say "always".

Using nextTick all the time uses a lot more time than calling back sync. pull-streams uses looper to iterate (i.e. loop) if the stream was sync, which blowing the stack.

@dominictarr
Copy link
Author

oh it just occured to me that you could call the function passed to map or reduce a functional function. It shouldn't have side effects, and that means not having any behavior after it's called (so no async)
calling it lambda fits in with this, because that comes from the "lambda calculus" which is all about functional programming. forEach kinda breaks this, because it doesn't return anything so obviously you are gonna use forEach when you want side effects.

Normally, in my code when I have a functional function I name in fun or fn. callbacks are named cb or callback or possibly next, and listeners are named listener or on{EventName}
So, I'm (implicitly) declaring the "time signature" of the function. This seems kinda like the type signature but represents the shape of the function in the time dimension rather than space.

I don't know if there is a type checker that checks this, but I feel that it would be very useful for writing asynchronous programs. (for the record, I don't object to runtime type errors, because they are pretty easy to fix. async timing bugs are way harder though)

@davidchase
Copy link
Owner

function passed to map or reduce a functional function

based on the fact that it shouldn't have a side-effect you could also call those functions passed to map/reduce a "pure function"

i havent personally used forEach for quite sometime now other than occasional removing DOM classes etc.

Its more evident to the developer and then the maintainers when you know what the function is doing and explicitly returning.

Obviously there are situations where you can not avoid side-effects especially when it comes to dealing with the browser and its quirks... and there are various nice functional approaches to dealing with isolating side-effects in a program.

@dominictarr
Copy link
Author

sure. there is still a time and place for side effects
(eg, listeners and callbacks generally have side effects)

also it would be fine to have a lamba that is called async, like pull.map uses, but it shouldn't it self have async side effects.

@dominictarr
Copy link
Author

it would be pretty neat to have a language which could enforce this kinda thing. in my opinion it this would be more helpful than regular type checks.

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 a pull request may close this issue.

2 participants