search

drewm / mailchimp-api

Super-simple, minimum abstraction MailChimp API v3 wrapper, in PHP
MIT License
1.99k stars 506 forks source link

SubscriberHash function should be static #279

Closed VincentLanglet closed 5 years ago

VincentLanglet commented 5 years ago

The subscriberHash function return

md5(strtolower($email));

There is no use of $this and no need to create a Mailchimp object with an api key. This function should be static to be used more easily.

drewm commented 5 years ago

It only makes sense to use alongside a Mailchimp object, so I don't see why that change is necessary.

VincentLanglet commented 5 years ago

It only makes sense to use alongside a Mailchimp object

@drewm I disagree. When you're using the webhook functionnality, you received request from Mailchimp and you don't need to instantiate any Mailchimp object. But you still should be allowed to get the subscriber hash from the email.

drewm commented 5 years ago

Why do you need to hash the email when receiving a web hook?

Keep in mind that what you're suggesting here is a breaking change.

VincentLanglet commented 5 years ago

When I receiving a webhook, I save the user data and his email and the hash. Cause in the user profil of our website, it is allowed for him to modify his email address.

If I need to make a request to Mailchimp, I can't use the email to compute the hash, because the email has maybe changed. I need the previous hash, that is why we kept it.

Keep in mind that what you're suggesting here is a breaking change.

Why is it a breaking change ? You still can call the function this way $this->yourFunction even if it is static.

drewm commented 5 years ago

Is it really that big of a deal to just instantiate the object?

You still can call the function this way $this->yourFunction even if it is static.

Ah yes, you're right. I was thinking static properties.

VincentLanglet commented 5 years ago

Is it really that big of a deal to just instantiate the object?

Is it really that big of a deal to just declare the method static?

I don't see like a good pattern to instantiate the object when you don't need it. Plus in my symfony project, I have to inject dependency (an MailChimpApiClient or directly the mailchimp key) just in order to compute the hash. So yeah, I kinda think it's important to not inject a service you could avoid to inject.

I really think that declaring this static function is an improvement without any negative consequences

drewm commented 5 years ago

Ok, let's do it.