Bitvested/ta.js

Why use async in atr calc

Closed this issue · 10 comments

The algorithm of atr is a simple mathematical calculation that does not use asynchrony at all.

This is so you can await the function. If you're doing lots of calculations it can block the event loop.

This is so you can await the function. If you're doing lots of calculations it can block the event loop.

but it doesn't make any sense. the calc is sync, with await or not, it will block the loop

Yes you're right. I do not remember why I have implemented it this way.
The first version of this package is already a few years back, I wasn't that proficient back then.
Do you know if there are significant time improvements to using sync functions?
Or are there other advantages that I'm not aware of?

Yes you're right. I do not remember why I have implemented it this way.
The first version of this package is already a few years back, I wasn't that proficient back then.
Do you know if there are significant time improvements to using sync functions?
Or are there other advantages that I'm not aware of?

Javascript “runtime” is single-threaded, that means synchronous task run in main thread and will not get into task queue. Most of your code is synchronous task, and has not asynchronous task. The await have no effect at synchronous task.

Yeah I know, but are there any other improvements to using synchronous functions as opposed to the current async functions. Changing the functions to synchronous functions would create breaking changes for people who already use the functions in combination with .then() and .catch(). I don't think I will change the package just for speed improvements. If you need speed you shouldn't use JavaScript in the first place. I recommend using this package only for rapid prototyping.

Yeah I know, but are there any other improvements to using synchronous functions as opposed to the current async functions.

It's not about improvements, it is syntax error!!!

So the function is not working as expected?

thanks for the functions @Vultwo .. I had used couple of those functions and great to have those functions all in one place.. great work.

Yeah I agree I had to go and remove async because async in your functions does not add any value and can be removed with no effect. async is used when you calling async code, it is a shortcut for Promises .. it does not magically make your code run faster or become asynchronous. you can only use async in functions if you would be awaiting the resolve of a promise from inside your function. Because you will not do that because you do not depend on any Promises then async and await is completely redundant and effectively do nothing. Also clients will not be impacted whether or not await is used because your functions does not return a Promise and so in all cases they have to wait for the function to complete. so when you mark a function with async clients dont need to await it because it does not return a promise either. I recommend to remove all async.. If , for example, you would like to implement something like that , you would need to change the functions to use Promises .. in that case , clients can do other things until the Promise is resolved. anyway , hope this helps for your next improvements and optimisation. thanks!

@nileio Thank you, I did not expect more users to find this an issue. I will remove the async reference from the functions as soon as possible.

I have updated the package. Async references are removed.
Let me know if you guys discover anything else.