0

I hope I can make clear what I'm struggling with :-) Here goes. I'm wondering how to implement the SRP on the following case:

There's a project. When finished, a contact has to be mailed with a survey in which he gives feedback on how the project went.

The software has a project-class. There's a procedure which loops through all projects. I have split off all code for mailing to a class called ContactMailer, which takes the project as a parameter, something like ContactMailer.AttemptMail(project);

But there are certain conditions in which a mail is NOT to be sent: the project is marked DoNotSurvey, or marked Challenged (= someone disputes that a mail should not be sent and an admin has to decide over this), and if there is no valid survey for this kind of project.

My question is: this check could be in a procedure like CheckMailConditions or something like that, but where does this procedure belong? Should it be in de ContactMailer? That feels somewhat off, although it IS a check if a mail should be sent. Or should it be a separate class? That sounds like SRP (a class that has one responsibility: check conditions) but this would lead to one class with one method, which seems overkill.

Or should I check these conditions even before calling ContactMailer.AttemptMail, since they are properties of the project? I'm kind of lost.

Thanks in advance for your thoughts!

1 Answers1

0

I don't think you're breaking the single responsibility rule by including the checks within your ContactMailer class. It would have the single responsibility of sending the mail if the project meets certain conditions.

A class with a single method isn't necessarily overkill, and there may be good reasons for abstracting the check logic into a separate class which aren't apparent to me from your question. However, from what you've said, I think your architecture sounds absolutely fine.

Just as an aside, if you did separate the check logic into a separate class, what would you call the class? MailConditionsChecker? In my experience, it's generally a bad sign if it's difficult to come up with a sensible noun to name your class.

net.uk.sweet
  • 12,444
  • 2
  • 24
  • 42
  • Thanks for the feedback, that helps me to be a little more sure about which choice to make. Good rule of thumb about the naming difficulty, I'll keep that in mind :) Your suggested name sounds like a name I would use. Just out of curiosity, what would be good reasons for abstracting the check logic into a class as you mentioned? One reason that comes to mind is if there's a lot of conditions which would make the code messy- but then you could also just put it in a procedure. –  Aug 08 '13 at 07:46
  • A couple of reasons I can think of for separating it out - perhaps another class needs to be able to share the same checking logic. Or perhaps you need to be able to run the mail class without the checking logic in certain environments / situations, perhaps when testing. – net.uk.sweet Aug 08 '13 at 09:30
  • By the way, welcome to stackoverflow! If the answer was useful to you, remember to click the green arrow to accept it. Thanks. – net.uk.sweet Aug 08 '13 at 20:27