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

MailClient, Service? #165

Open
pas opened this issue Dec 17, 2012 · 0 comments
Open

MailClient, Service? #165

pas opened this issue Dec 17, 2012 · 0 comments
Assignees
Milestone

Comments

@pas
Copy link
Contributor

pas commented Dec 17, 2012

Review:

I wouldn’t consider the mail client a model. In your model, there is no such thing as a mail client. This is a traditional service and I wouldn’t put it into the Model package. The separation between the client and the helper is also very confusing. A constructor should not be called “setup”. Then every mail has the same subject, which welcomes the user. This should definitely not be hardcoded in the “helper”. I would redesign that quite heavily.

@ghost ghost assigned DKPillo Dec 18, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants