refactor connection usage

[makasim]

I've made some changes in ChargifyConnector useage.

Now all object should receive connector as a fitst constructor parameter.

This change makes the library more flexibal.

You can have as many connectors as you want (two connectors: test and prod for example).
Also it makes the library easy to test.

I would very happy ifyou add this changes to your branch.

[jforrest]

Thank you!

I will have some time this weekend to integrate your changes. I will shoot
you an email once I finish.

On Thu, Mar 24, 2011 at 10:22 AM, makasim <
[email protected]>wrote:

I've made some changes in ChargifyConnector useage.

Now all object should receive connector as a fitst constructor parameter.

This change makes the library more flexibal.

You can have as many connectors as you want (two connectors: test and prod
for example).
Also it makes the library easy to test.

I would very happy ifyou add this changes to your branch.

Reply to this email directly or view it on GitHub:
#7

[makasim]

Sometime past and there is not any news from you.
I would like to hear your feedback to my changes.
Are you going to merge?

Thanks!

[ziemkowski]

I was just going to create an issue saying there was no way to set connection information without editing the source files, which makes updating more prone to issues. Your changes fix that issue since I can just have a custom class that returns a connector that any other file can then use without knowing the connection information and without editing any of the source files.

Read the diffs and they look good. +1 for pulling in these changes.

One question though, since this change isn't backwards compatible, why not take the opportunity to make the factory methods actually static? For example, you still have to create a subscription with a connection, and then replace the instance with one returned by getByID(), getByCustomerID(), or getAll(). Would definitely be cleaner to have ChargifySubscription::getByID($connector, $id); for instance.

Going even further, I could see swapping the parameter order so $connector is last and optional with a static ChargifyConnector::$default_connector used if null (which could be populated by say the last created connector or via a method that makes the connector default).

[makasim]

I don't like the idea of factory method. Because:

1. It makes dependency between connector and api classes less clear.
2. It is library and if it so it should be as much flexible as it possible,
3. The default connector can be handled by an application api layer which stands above the lib.

But I see the way to archive what you want. I think it can be a ChargifyConnectorManger (like it in Doctrine) which can be used by api classes.

Thanks for support.

[ziemkowski]

I shouldn't have used the term factory, I was using it loosely to mean they're creation methods. It just seems odd that they're instance methods creating new instances, instead of say a static method.

So I have to create an instance, then replace it with the method's new instance, and then when I call some other method, the connector creates yet another instance.