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

Feature request: make async / sync functions explicit #39

Open
jadonk opened this issue Apr 25, 2018 · 2 comments
Open

Feature request: make async / sync functions explicit #39

jadonk opened this issue Apr 25, 2018 · 2 comments
Assignees

Comments

@jadonk
Copy link
Member

jadonk commented Apr 25, 2018

From @psiphi75 on April 24, 2016 0:31

In node the file-system operations are by default asynchronous. Some file operations allow for synchronous operation, blocking the code, obviously. This is explicitly defined by labeling synchronous functions by appending 'Sync' to the function name. Labeling a function 'Sync' is highly advantageous because it shows the programmer that this is a blocking function.

Bonescript goes against this convention. It overloads read/write operations to perform both sync and async, depending on weather a callback was supplied. I suggest splitting this out. There are two ways of doing this, the non-breaking way and the breaking way.

I would be glad to implement these changes, but I would need your okay with how to continue.

Option 1: breaking
This option would break existing implementations using bonescript. But it would make using bonescript clearer, since sync functions would be labeled and the programmer would know if the function is blocking or not.
Features:

  • Add a warning message (or error) to all dual-purpose functions, that it is now asynchronous.
  • Create a sync function for each dual-purpose function.
  • The old function would not return a valid value for a Sync operation.

Example: analogRead(pin, [callback]) -> value would become analogRead(pin, callback) (always returns undefined) and a new function would be created like: analogReadSync(pin) -> value.

Option 2: non-breaking
This option would be non-breaking, but we could add a message for users for them to migrate to option 1.
Features:

  • Keeps the existing function as-is.
  • Add a warning message to all dual-purpose functions, that it is now asynchronous.
  • Create a sync and async function for each dual-purpose function.
  • Provides a possible migration path to option 1.

Example: analogRead(pin, [callback]) -> value remains, but when called, it is suggested to use one . The following two functions are added:

  • new function: analogReadAsync(pin, callback).
  • new function: analogReadSync(pin) -> value.

Copied from original issue: jadonk#125

@jadonk
Copy link
Member Author

jadonk commented May 21, 2018

I actually don't want this "fixed" in any way that might make old code not run. I'm not even very excited about presenting the non-delcarative syntax as depreciated. I'm open to ideas that make it possible to annotate the call as synchronous or asynchronous for those that want to make it clear and even updating the bone101 examples, but I don't like any of the proposals provided.

@psiphi75
Copy link

I have to say that I agree. In the meantime Node.js and JavaScript have evolved quite nicely. The Node.js callback syntax I suggested above is old-school. I propose another solution that does not change the current implementation, but provides a modern JavaScript programming environment for those who want it.

Option 3: Promises combined with submodules

const b = require('bonescript/asyncFuncs');

b.analogRead(pin)
        .then(result => console.log(result))
        .catch(err => console.log(err);

// Or with Node.js v8.0+ (should be encapsulated in try-catch)
let result = await b.analogRead(pin);

Notes:

  • This will not change the existing Bonescript functions if loading the module using the current method.
  • The asyncFuncs submodule would just be a wrapper around the current Bonescript functions.
  • Debian stable only has node v4.x at the moment. Hence async/await is not supported, however, v4.x supports Promises. Node.js v8.0 supports async/await functions.
  • Since async/await functions are just Promises in disguise, no extra code is required to create the async/await functionality, it will just work as shown above for those who are running Node.js v8 or later.
  • The submodule will be exposed on require('bonescript');

I recently received my BeagleBone Blue and hope to get back into this soon. If there is interest on this concept I can implement it.

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

No branches or pull requests

3 participants