AlgoTrader / betfair-sports-api

Library is discontinued
MIT License
55 stars 15 forks source link

Small mistake in the documentation and request #1

Closed giacecco closed 12 years ago

giacecco commented 12 years ago

Hi, in most of your examples you suggest invoking by doing:

inv.execute( function(err, inv) {...} )

but I understand it should rather be

inv.execute( function(err, res) {...} )

Please also clarify how keepAlive() is supposed to be used. E.g. is it necessary in any case, or any of the other function calls keeps the session alive anyway?

In the meantime I am flattring you (https://flattr.com) to thank you for the work done so far. Thanks!

Giacecco

AlgoTrader commented 12 years ago

Hi, thanks for questions.

First, when writing a function you can name parameters as you want. inv.execute( function(err, inv) {...} ) and inv.execute(function(err, res) {...} ) are absolutly identical. In the example, you execute the Betfair invocation and provide a callback to be called when invocation is finished. It does not matter res or inv used, I use both.

Second. keepAlive is the absolutly useless Betfair invocation in real apps, but it is good for 1) educational purposes as it is simple, not throttled and never charged 2) testing network performance, reasons are the same

In fact, keepAlive must be sent once in 20 minutes if no other calls were issued in the time. I cannot imagine a real world app that does not call a single invocation in 20 minues.

I hope it helps. Anton

giacecco commented 12 years ago

Thanks Anton

Hi agree the name is irrelevant, but by using the same name on the right and on the left the reader will naturally think that the callback function is passed the 'same kind of object', that is very very misleading! On the left inv is a... I don't know, on the right inv is a results object.

About keepAlive, beyond what you describe, isn't keepAlive necessary if one needed to keep the session alive when not in condition to do anything else, e.g. because no decent bet can be placed?

G.

AlgoTrader commented 12 years ago

It is not the same kind of object, it is the same object. In javascripts name of object is reference. For example

var myObject = { name: 'Anton'; } var first = myObject; var second= myObject;

Now, first and second are references to myObject. changing first you change second and changing second you change first.

It does not matter how you name parameter, res or inv, it is still references the inv object before the dot. So in the code

inv.execute( function(err, invocation) {...} )

inv and invocation is the same object referenced using different names.

AlgoTrader commented 12 years ago

Hope it should help you:

C:>node

var first = {name:'Anton'} undefined var second = first undefined second.name = 'John' 'John' console.log(first) { name: 'John' } undefined first { name: 'John' } first==second true

Summary: we have a single object but two names for it. It's very common in JavaScript.

AlgoTrader commented 12 years ago

I think questions are resolved

giacecco commented 12 years ago

Thanks Anton I believe I know JavaScript sufficiently. My point is about the design of your library. Do you mean that you use 'inv' on both left and right because they refer exactly the same object? For example, would the code below work?

var inv = session.getAllMarkets({ eventTypeIds: [1, 2] });
inv.execute(function(err, inv) {
  inv.execute(function(err, inv) { // do nothing });
});
AlgoTrader commented 12 years ago

It will. It's meaningless though. Any invocation can be 'reinvoked' but normally only in case of error. Coding in this style will cause "callback hell" with a number levels of identing.

I strongly recommend to use anyc lib with code like this

inv.retries = 0;
async.untill(function() { 
         return inv.success;
     }, function(cb) { 
    ++inv.retries;
    if(inv.retries>=10) {
        cb('maximum retries reached');
        return;
    }
    inv.execute( function(err,res){
        if(!err)
           inv.success = true;
        cb(null);
   })
})
AlgoTrader commented 12 years ago

I forgot, there is inv.isSuccess() function, it can be used as first parameter or async.until

the code simplifies to:

inv.retries = 0;
async.until(inv.isSuccess,
    function(cb) { 
        ++inv.retries;
        if(inv.retries>=10) {
             cb('MAX_RETRIES_REQCHED');
             return;
        }
        inv.execute( function(err,res) {
            cb(null);
       });
})
AlgoTrader commented 12 years ago

I added inv.retries that counts inv.execute() calls. Now it's

async.until(inv.isSuccess,
    function(cb) { 
        if(inv.retries>=10) {
             cb('MAX_RETRIES_REACHED');
             return;
        }
        inv.execute( function(err,res) {
            cb(null);
       });
})

It means the following. until inv.isSuccess() returns true, run inv.execute(). Each inv.execute() increaments inv.retries property. If we reached MAX_RETRIES (10), break until with error.

At last the code is simple and easy to read! I need retry logic myself, so I want it to quite simple and clear. I never use execute to run a successful invocation, always create a new one, but it's personal preference