node-vkontakte icon indicating copy to clipboard operation
node-vkontakte copied to clipboard

Use OOP instead of closures.

Open AlexeyKupershtokh opened this issue 12 years ago • 7 comments

I see your lib utilizes closures heavily just in order to make syntax shorter. But I think that instantiating a class could give cleaner code, additional easy to implement things like I mentioned in https://github.com/stevebest/node-vkontakte/pull/1#issuecomment-11366871 and compliance with the official API that uses VK.api().

AlexeyKupershtokh avatar Dec 14 '12 07:12 AlexeyKupershtokh

When in Rome, do as Romans do. I used closures not for syntax, but because it's the way Node modules are expected to be. Well, at least Roman emper---I mean @substack writes them that way.

The substack pattern — module.exports = function () {}. So module should be so simple that it can be described by one function.

That said, if I wanted to try something masochistic and port official VK API to Node, I'd do it. But I don't wanna. :)

stevebest avatar Dec 14 '12 08:12 stevebest

Also, https://github.com/stevebest/vk-client

stevebest avatar Dec 14 '12 08:12 stevebest

Also, https://github.com/stevebest/vk-client

Thanks! That seems more convenient for me.

AlexeyKupershtokh avatar Dec 14 '12 08:12 AlexeyKupershtokh

Aaaaand, I'm probably not going to support it. It's twice as long and doesn't even speak Node streams, bleh! :)

stevebest avatar Dec 14 '12 08:12 stevebest

Regarding the style, supporting 2 api versions (full and shorthand) is still achievable:

// shorthand api
module.exports = function(key, secret) {
  var instance = new Client(key, secret);
  return function() {
    instance.api.apply(instance, arguments);
  }
}

// full api
var Client = module.exports.Client = function(key, secret) {};
Client.prototype.setRequest(req) { this.request = req; };
Client.prototype.api = function() { ... };

Is this idea clear to you? This method will preserve external simplicity but also is able expose details for those are looking for them.

AlexeyKupershtokh avatar Dec 14 '12 09:12 AlexeyKupershtokh

This will give 2 call styles: the old one

var vk = vkontakte('', '');
vk('method', ...);

and the new:

var vk = new vkontakte.Client('', '');
vk.setRequest(request.defaults({proxy: ...}));
vk.api('method', ...);

AlexeyKupershtokh avatar Dec 14 '12 09:12 AlexeyKupershtokh

I was thinking about making it more request-compatible in terms of API, like

vk.defaults({proxy: ...}) // delegate `request` stuff to `request`
vk.get('method') // when you actually need a GET
vk.post(...) // support streams for e.g. uploading videos and shit
vk('method') // maybe even be smart and pick GET or POST depending on `method`?

stevebest avatar Dec 14 '12 09:12 stevebest