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

Unified parallel and serial registrations (type is determined by the … #9

Closed
wants to merge 1 commit into from
Closed

Unified parallel and serial registrations (type is determined by the … #9

wants to merge 1 commit into from

Conversation

tweinfeld
Copy link

It might be beneficial to allow a single ".register" method for both linear (promises) and parallel "handlers"

@sefininio
Copy link
Contributor

Hi,

Thanks for taking the time to implement this. However I am not going to accept your PR.

It's important for me to explain why, though.

First, I don't like the coding style. I prefer the code to be readable and your code, while it is obviously understandable, it is not easy to follow through it or easily debug it, so while the functionality you propose is nice to have, it does not merit the code be unreadable or unmaintainable.

Second (and this is the major issue I have with the code), is that it breaks the functionality (I think, not sure though). As far as I know, JS doesn't let you know the function return value unless you invoke it, which breaks the TickerSrv delay parameter. As long as the delay is 0, your code works. However, if the delay is larger than 0, then your code breaks it because it has to invoke the function to know it's return value (or throw an error?).

@sefininio sefininio closed this Jun 21, 2015
@tweinfeld
Copy link
Author

No problem 😃 - good job on this service!

  1. Readability: I probably can't effectively argue with that. However, please consider that, as with beauty, readability is largely in the eyes of the beholder. I try to promote a slightly more "functional" coding style in which less imperative decisions are made (if (interval === undefined)...), which should improve its robustness.
  2. Functionality: I've made isLinear redundant by running the tickHandler function inside a $q promise's .then, which handles both synchronous functions (returning a value other than a Promise) and asynchronous functions (which utilize Promises in their return value). delay is effective only on the first execution following a register call (as I believe was your original intention?). It is applied correctly regardless of the function's return value.

I'd be happy to assist if you'd like to further explore my PR.

@sefininio
Copy link
Contributor

It's not the functional programming that bothers me, it's the multiple inner inline anonymous functions that are defined and called as a one liner. This, to me, is less readable than say, defining the functions as members of the service (as private if applicable) and calling them as the algorithm progresses.
Sure, you'll need to make sure the proper function scope is maintained but other than that it'll add to the code readability.

I guess it's a matter of code styling, a matter of taste.. Since at the end of the day the code works :)

@tweinfeld
Copy link
Author

IMHO not every function deserves a name, let alone alone their own service member references (interface placement).

Functions are the "bread & butter" of, well, functional programming, and therefore are extremely ubiquitous in it. They're used for many purposes - some narrowly utilitarian (like those used for scope isolation, lambda expressions, monads) while others more business-expressive and reusable. Assigning names to every function in a highly-functional code could actually impair readability and result in a very cumbersome verbose mash - all without gaining any benefits. Think what would happen if you had to name each ".map" iteratee function? Or how your code would look should you named every function in a y-combinator implementation?

Even more importantly - FP is more than just a code-style choice, it has the power to make your code more robust, resilient, reactive, and, in my view, it actually changes the way you think of software as a whole by shifting your focus from the mundane of small-time implementations (mostly manifested in ifs and switches) to the bigger picture (like event-reactions, data transformations and application business).

If you want to dig more into FP, I warmly recommend BaconJS/Kefir and Cycle

🍺
Tal

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.

None yet

2 participants