Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring: Removal of collections #439

Merged
merged 2 commits into from
Oct 9, 2017

Conversation

jopanel
Copy link
Contributor

@jopanel jopanel commented Oct 4, 2017

Collections should be optional. Not all OOP frameworks utilize collections. Nor people who wan't to create a procedural script.

Collections should be optional. Not all OOP frameworks utilize collections. Nor people who wan't to create a procedural script.
@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 4, 2017
@jopanel jopanel mentioned this pull request Oct 4, 2017
@shonjord
Copy link

shonjord commented Oct 5, 2017

@jopanel This PR to me makes more sense, you're just removing the collections, that's fine, because the library can still type hint using arrays 👍 .

@jopanel
Copy link
Contributor Author

jopanel commented Oct 5, 2017

Based on my previous PR and our conversation I think this is the better way to go. Almost every occasion there is no collections class that I have been apart of in an in-house setting. However many businesses utilize symfony, laravel, magento, etc. that has a collections class. It will not be a problem to utilize a collection wrapping an array if needed by the user. If a case were to ever arise.

The objects for "new To" and such I understand will work good for debugging and validation. Standard OOP practices and such make them nice. My original concern before was that there was going to be no validation for the objects and that they were uselessly being called too. However that's not the case.

Also we must not forget category! It is super important. It helps so much to know what email got sent to what person. We send emails for: invoices, shipping confirmations, registration, reactivation, etc. It is very important to know what email got sent when I am asked by sales department "Hey Joe customer X didn't receive the Y email". Same with our marketing mass emails we send out. I can use the API to pull statistics on the category back to our backend which I do.

@thinkingserious thinkingserious added difficulty: medium fix is medium in difficulty hacktoberfest labels Oct 9, 2017
@thinkingserious thinkingserious merged commit 981600a into sendgrid:master Oct 9, 2017
@thinkingserious
Copy link
Contributor

Hello @jopanel,

Thanks again for the PR!

We want to show our appreciation by sending you some swag. Could you please fill out this form so we can send it to you? Thanks!

Team SendGrid DX

@thinkingserious thinkingserious added hacktoberfest difficulty: easy fix is easy in difficulty and removed difficulty: medium fix is medium in difficulty labels Oct 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy fix is easy in difficulty status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants