Single file held state

[Haziba]

Note to tester: Rejigged everything so state can be both held by the client and overwritten in each method call

[AdriVanHoudt]

This is a way nicer implementation :D 👌

• Would it make sense to make api like this:

  const Cronofy = require('cronofy');
  const cronofyClient = new Cronofy(options);

  This would allow you to put the methods on the prototype which makes it even faster. In the cronofy function (https://github.com/cronofy/cronofy-node/pull/23/files#diff-1fdf421c05c1140f6d71444ea2b27638R13) you would have to do a this.config = config and where you use the config you'd have to use this.config Does that make sense?

• It seems like it isn't possible to pass next_page to readEvents anymore (see https://github.com/cronofy/cronofy-node/pull/23/files#diff-a973e3c7958cb2dada1bee5d70a58e2eL12) which is something we definatly need

[Haziba]

Sound like some good changes to make, from looking at the next_page situation however I feel like the SDK would benefit from an iterator so that the pagination is handled from inside the Cronofy object?

Also I'm hesitant to make a breaking change to the error handling but currently we're passing back the whole response as the error, whereas I believe it would be better to just pass back the status code and the response body, so that whoever is using this client will just get the information they need and have an easier time working out what the issue is with a failing call.

What do you think to both these points?

[AdriVanHoudt]

• Handling next_page yourself seems fine. You'd need to keep track of it and provide a simple api to ask for the next page then.
• Decent errors would be great! I now need to delete error.raw to get some valuable errors :P

[Haziba]

@AdriVanHoudt How's that?

[AdriVanHoudt]

Are you sure that the current implementation does the prototype thing? I think you are still setting all the methods on creating a new instance. Instead of reusing them for each instance. What you want to do (I think) is

var cronofy = function cronofy(config) {
     this.config = config;
};

cronofy.prototype.readEvents = function (...) {...};

Does that make sense or am I missing the ball here?

[Haziba]

Ah sorry I misunderstood! JavaScript isn't my primary language so I'm not quite so knowledgable on prototypes and the like, so thanks very much for your help :)

Does this look alright now?

[Haziba]

@AdriVanHoudt How's that?

[AdriVanHoudt]

Seems good 👍