joostvunderink / angular-jsonrpc-client

A JSON-RPC 2.0 client for AngularJS
MIT License
26 stars 20 forks source link

fixed result = false bug, added jsonrpc batch #12

Closed GerritK closed 8 years ago

GerritK commented 8 years ago

fixed a bug where result equals false throws an error

added jsonrpc batch support

joostvunderink commented 8 years ago

Wow, thank you Gerrit for this change. I'll add some unit tests for the new batch functionality and then release it!

joostvunderink commented 8 years ago

I've looked at the code. What made you decide to specify an individual then for each batch.add()? I would have coded it something like this:

var batch = jsonrpc.batch();
batch.add(call1, args1);
batch.add(call2, args2);
batch.send()
.then(function(results) {
  // results[0] contains the return value of call1
  // results[1] contains the return value of call2
});

Note that I haven't used batch jsonrpc requests, so I'm not sure what the best/right approach is here.

GerritK commented 8 years ago

I thought it would be intuitive to have an individual then for each add. With your method you must keep track of the request id, I think. Especially if you build your batch in an async way it gets more and more complicated. it would be an compromise if you could use both ways, maybe via configuration?

edit: another important point is the error handling. each request can succeed or fail by itself. so you have to do the error check by yourself in the then function

joostvunderink commented 8 years ago

I think it makes sense what you say. I'll write an example and some unit tests and then release a new version.

joostvunderink commented 8 years ago

Hi Gerrit,

I've released your code as is, functionality-wise. I've added tests and I've rewritten a few for-loops into forEaches. And I've added your name to README.md :-)

Thanks again for your contribution!

Kind regards, Joost.

On Thu, Feb 4, 2016 at 8:32 PM, Gerrit Kaul notifications@github.com wrote:

I thought it would be intuitive to have an individual then for each add. With your method you must keep track of the request id, I think. Especially if you build your batch in an async way it gets more and more complicated. it would be an compromise if you could use both ways, maybe via configuration?

— Reply to this email directly or view it on GitHub https://github.com/joostvunderink/angular-jsonrpc-client/pull/12#issuecomment-180015095 .