Singleton VS new Object

[usama33]

Hi,

I just realized that api is singleton. What If user wants to create multiple binance api objects based on different api key/secrets? I did few changes and it works now with 'new'. If you want I can create a pull request for you or following is what I did;

Replaced this;
module.exports = function() {

with
function Binance() { if (!(this instanceof Binance)) { return new Binance(); }

And added
module.exports = Binance;

Usage
var Binance = require('node-binance-api');
module.exports = class MyWrapper {
constructor() { this.binance = new Binance(); }
}

[jaggedsoft]

Thank you for doing this! I'm assuming this is a breaking change? If so, I will schedule this for a release soon at version v0.5.0

[springjben]

      module.exports = class MyWrapper {
        constructor() { this.binance = new Binance(); }
      }

With the above statement, how do I access the binance information outside of this class?

I am using socket.io with sessions, I can create a class, but how does one change the options for each individual user?

[usama33]

It doesn't necessarily mean you should be creating inside the class.. var myBinanceObject = new Binance(); myBinanceObject.options({ 'APIKEY': apiKey, 'APISECRET': apiSecret, useServerTime: true }, null); above should work as well... If you wanna call from within class... Then in any function of the class you can call with "this.binance"

[jaggedsoft]

@dmzoneill I am having trouble implementing this, but since we just renamed trades to aggTrades now would be a great time to bump the version number up to 6.0. Would really appreciate your help with this if you have time

[dmzoneill]

Cool, im not awesomely familiar with nodejs... more of a web/jquery guy..

It cant be that hard to figure out.. ill see if i can find an hour over the weekend..

the misses quit her job.. shes around and taking up my time atm..... 😉

[Ionut-Milas]

hi @jaggedsoft Is the deployed?
I'm getting a "Binance is not a constructor" error.

[jaggedsoft]

@Ionut-Milas No, I have attempted this, but I am unable to get it to work as desired. Someone else will need to do it, or I will revisit it later

I want this feature

[usama33]

@jaggedsoft here is the diff of what I did. Let me know If you are unable to implement this.

https://www.diffchecker.com/cYkDMgwX

[jaggedsoft]

@usama33 Fantastic mate, that is very helpful. Thank you!! I'll issue a new release and give you full credit

[jaggedsoft]

Available in v0.7.0 57706f6
Thank you again!!

[bmino]

As far as I can tell, the instances are working great!
However, does this break the ability to share the internal depth cache?

For instance, let's say an application (using node-binance-api as a dependency) opens a few websocket streams using the depthCache() method which populates the internal depthCache object. Now, two parts of this application want to require node-binance-api and read from the depthCache. Is this no longer possible without maintaining another local copy of the depthCache in the application?

[jaggedsoft]

@bmino I'm not sure the best way to do this. What I had considered doing in the past was packaging all websocket functions in their own module. Since only account balance and trade execution updates require a key

[bmino]

What if the following block of code is removed? It forces instantiation.

if (!(this instanceof Binance)) {
    return new Binance();
}

Without that block, I think you can create both instances and singletons like this:

let api = require('node-binance-api');
let instance = new api();
let singleton = api();

Here is a gist of my local simplified example testing this:
https://gist.github.com/bmino/a52998e6c184437ac0e524cf8ff06daa

[bmino]

I submitted PR #235 which will revert the default behavior of

const api = require('node-binance-api');

to return a singleton, but users can create an instance by the steps I mentioned above. Can someone test this with the newer import syntax?

[bmino]

There are some subtleties I missed in simplifying my gist test case. Mainly the use of assigning all variables to the "this" scope of the function export. I am now testing with the entirely of the library locally (instead of a simplified version) to ensure I don't miss anything.

But, it appears that the api credentials used in the test script are invalid now?

[dmzoneill]

Yes binance invalidated all keys , im on business travel till sunday. Not sure if will get to it before then

@jaggedsoft

[jaggedsoft]

I don't have a test account to issue new sandbox keys, sorry. I suppose we will have to be patient :) Thanks guys

[bmino]

I submitted PR #252 to address the issues of shared variables between singletons.

Testing:

• Passes project unit tests
• Passes project linting standards
• Tested in my project which restores it to full functionality (using the singleton pattern).
• Tested against this gist https://gist.github.com/bmino/bbb4669c7c40947166c9b1fc1d05ad8a