Be explicit, throw an exception when a request fails

[jonathan-s]

In the following lines you simply return what the error is. It's better to throw an exception. Explicit is better than implicit.

https://github.com/ActiveCampaign/activecampaign-api-php/blob/master/includes/Connector.class.php#L197-L205

[pevans]

I agree in principle. I am, however, hesitant to change the current behavior considering that there is likely code written which expects to do as it is now. With that said, we are working on a new version of the API which should take the approach of an exception in this instance (and similar ones).

[jonathan-s]

I'll give you two reasons why I still think you should change the code. Developers rely on versioning. As long as they pin to a specific version. They won't be affected. If you think this is a major change. Give a major version change to convey it. Aka semantic versioning.

Second reason, it'll be a boon to developers who stumble upon the issue by mistake, as it'll be easy to find in the logs. Developers used by the former approach will also easily find out where things went awry.

After all there isn't a good way to recover from this except retrying.

On Thu, 29 Sep 2016, 20:35 Peter Evans, notifications@github.com wrote:

I agree in principle. I am, however, hesitant to change the current behavior considering that there is likely code written which expects to do as it is now. With that said, we are working on a new version of the API which should take the approach of an exception in this instance (and similar ones).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ActiveCampaign/activecampaign-api-php/issues/57#issuecomment-250568643, or mute the thread https://github.com/notifications/unsubscribe-auth/ACBsEuIODzDNNYINs7wah6AeZoYuUoVyks5qvBMLgaJpZM4KKSoQ .

[bartboy011]

@jonathan-s Take a look at #85, I know it's been now 15 months since you opened this issue but your requested changes are finally being implemented.

[jonathan-s]

Thank you! I've changed jobs since then and no longer use ActiveCampaign, but appreciate that it's been implemented! I'm sure others will appreciate it!

Jonathan Sundqvist Twitter http://www.twitter.com/ecologythinking | Linkedin http://se.linkedin.com/in/jonathansundqvist

2017-12-19 22:33 GMT+01:00 Joshua Bartlett notifications@github.com:

@jonathan-s https://github.com/jonathan-s Take a look at #85 https://github.com/ActiveCampaign/activecampaign-api-php/pull/85, I know it's been now 15 months since you opened this issue but your requested changes are finally being implemented.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ActiveCampaign/activecampaign-api-php/issues/57#issuecomment-352892773, or mute the thread https://github.com/notifications/unsubscribe-auth/ACBsEuUEXXTCNlc_0oYptwzDM41pdSmGks5tCCuPgaJpZM4KKSoQ .

[cristiangrama]

85