baconjs / bacon.js

Functional reactive programming library for TypeScript and JavaScript
https://baconjs.github.io
MIT License
6.47k stars 330 forks source link

observable.take() documentation is circular hence offers no definition #612

Open cefn opened 9 years ago

cefn commented 9 years ago

The documentation provided for Bacon at this page could be infinitely improved by specifying the operations performed in javascript terms.

For example, observable.take( ) and it's related function calls observable.takeX() all refer to the same 'take' operation without ever defining what take means, and no javascript-oriented contract is described.

"observable.take(n) takes at most n elements from the stream" 

Currently there seems to be an expectation that the developer already knows everything about Bacon and operations are therefore described in Bacon terms, meaning it's impenetrable from the outside . One specific gap which could help people trying to fill in the gaps would be specifying the return type for every operation. In this case...

However crazy some of these alternatives might be, as a developer outside the project, (or possibly new to javascript), you still have to eliminate all these possibilities and somehow invent or introspect what the operations do which is extremely hard. I'm certain people with project knowledge would be able to provide this extra precision and I believe it is a requirement for seamless adoption.

Out of interest, what does observable.take(...) do?

pnex2000 commented 9 years ago

I agree that some bits are a bit vaguely described. The way I read you two things stand out: a general description of how Bacon works and API documentation of individual methods. I'm not really in a position to say whether general documentation and tutorials are good enough, but I find that the API documentation is most of the time giving the information I seek. On the top of my head all methods return a stream except subscribe and its derivatives, which is noted, so repeating this wouldn't be useful. But it's good to get concrete suggestions and it's good to hear experiences of the documentation.

As for take, it says "takes at most n elements from the stream.", which more formally means that take takes n elements or all the elements up to the end of the stream, whichever is fewer and then just propagates the end event. Looking at the documentation, it could be useful to add the additional detail and rearrange take to be first before its variants.

cefn commented 9 years ago

Thanks for exploring this documentation improvement. The fact that all methods return a stream is non-obvious I think, and would ideally be recorded against each method, detailing the nature of the returned stream.

The phrase 'takes...elements from the stream' is quite misleading. Although I can see the value of the method itself having a short evocative name, the documentation has to resolve the ambiguity from using this word in an unconventional sense. 'Taking' can only really be interpreted in colloquial english as removing things from the stream, which it does not do. If colloquial english is not intended, then I believe a more precise type of language, suited for a method definition, should be used instead.

In the end, I've learned take() doesn't take any elements from the source stream at all, it...

creates a new stream which _notifies_ at most n future events 
generated by the source stream. The stream ends after n events 
or when the source stream ends.

...but figuring this out backwards from the implementation is a hard road.

raimohanska commented 9 years ago

Guys, would you like to submit a PR to improve the docs. I find more value in trying to fix things than in discussing how wrong things are :)

pnex2000 commented 9 years ago

I've created a PR: https://github.com/baconjs/bacon.js/pull/623