Hacker News new | comments | show | ask | jobs | submit login

While I don't like the name of the service object "CreatesContact", I disagree with the statement that the sending of an email should reside within the create_contact! method. The author's intent is to shield any new developers from methods that say one thing and do another, yet he adds email delivery into the create_contact! method without informing the developer what may happen (unless some magic flag is set somewhere).

A simpler, more contrived example, would be to create a new function create_and_send_email! and place the creation logic and email sending logic in there. And now, when the developer uses the service object, she has the ability to choose between just creating the contact, or creating it and sending an email.

    class CreatesContact
      def initialize(contact)
        @contact = contact

      def create_contact!

      def create_contact_and_send_email!

In the Obvious architecture, it would be something more like..

    class CreateContact
      def initialize contact_jack, email_jack
        @contact_jack = contact_jack
        @email_jack = email_jack
      def do input
        # validate input
        contact = Contact.new
        contact.populate input

        contact_jack.save contact.to_hash

        email = ContactEmail.new
        email.populate contact.to_hash

        email_jack.send email.to_hash

With that structure you can call CreateContact with:

    action = CreateContact.new
    result = action.do ContactJack.new, EmailJack.new
In that structure you can totally test the action and logic without hitting the db or the mail system at all. Your ContactJack and EmailJack can be easily swapped out for various pluggable data stores. Fileystem, MySQL, Mongo, Postgres, Cassandra, could be swapped out for the ContactJack. EmailJack could send through standard mail servers, SendGrid, Amazon AWS, Mandrill, etc.

Obvious is on github, you can read more at http://obvious.retromocha.com

I agree with your comments on the class name, it should probably be ContactCreator. But I think having a method named create_contact_and_send_email is kind of code-smelly. I'd most likely do something like:

    create_contact!(:send_email => false)

Yeah "CreatesContract" is a weird name. ContractCreator, or ContractRepository (so it can do more than just create objects) makes more sense to me.

Plus he should probably just break the send_email method into it's own method, which he can then call depending on if create_contact returns successfully.

why not use an observer to send the emails?

Applications are open for YC Winter 2018

Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | DMCA | Apply to YC | Contact