Hexagon / node-telldus

Node bindings for telldus-core
Other
34 stars 10 forks source link

Should callback signature for async methods be more like node with an 'err' first argument? #15

Closed kmpm closed 10 years ago

kmpm commented 10 years ago

I would like the callback signature of the async events be like

callback(err, <something depending on method used>)

Where err should be an Error object with the message taken from ::getErrorString if the returnValue was above 0 and null if everything was ok. Just like you can expect from many other node modules.

I don't know what this would actually take to implement but it could be done as a wrapper in the call in telldus.js

Hexagon commented 10 years ago

Conforming to standards is nice! If you feel like doing it, do it :)

I think it's ok to do it in telldus.js, no need to complicate the C++ wrapper.

Check the returnValue for being 0 (TELLDUS_SUCCESS) before consulting getErrorString, that way the extra call to getErrorString won't have to be done at each run.

Hexagon commented 10 years ago

According to http://developer.telldus.com/doxygen/group__core.html#gaa7a350c175a2fa53f0d541a0eae12901 , getName, getModel and all the other functions returning a string, indicate error by returning an empty string. That'll have to be taken care of too as they return no integer value.

kmpm commented 10 years ago

Ok. So for example if ::getNameSync would have returned an empty string, ::getName should generate someting like callback(new Error("Nothing to get!"));

Hexagon commented 10 years ago

Sounds good :)

kmpm commented 10 years ago

How should booleans like in #16 work when it's done. Should false generate error in the callback?

Hexagon commented 10 years ago

Docs says true on success, false otherwise. So yeah, false should generate an error :+1:

nilzen commented 10 years ago

@kmpm will you keep the error number in the error "response" or will you just pass the friendly string from tdGetErrorString?

kmpm commented 10 years ago

So far only the message since I use the generic Error object. But I could create a class that inherits from Error and add a property for the error code as well. Should I do that and call it TelldusError or something?

nilzen commented 10 years ago

I would like the error code as well. So a TelldusError would be nice. It gives a simpler way to localize the error messages on an int instead of a string.