# A class that uses a few other classes to generate an email

I seem to find my self writing a lot of classes like the below where there seems to be some kind of send method that just calls a bunch of other methods. Is this bad practice? Is there a more object oriented way to do this?

The below class uses User, UserDetail (preference object), and a custom Mailchimp api client. It basically goes through creates a campaign, adds the html body to that campaign and then sends it.

# Generate and send mailers
class Mail
def initialize(user_id)
@user = User.find(user_id)
@detail = @user.details
@client = @user.mailchimp
@campaign_id
end

def send
create_campaign
create_html_content
send_campaign
end

private

def create_campaign
params = @detail.create_campaign_params
@campaign_id = @client.create_campaign(params)
end

def create_html_content
params = @detail.create_content_params
@client.create_campaign_content(@campaign_id, params)
end

def send_campaign
@client.send_campaign(@campaign_id)
end
end

• What is @campaign doing as the return value from the initializer? – David Aldridge Jul 29 '17 at 11:42
• I think you meant@campaign_id, it doesn't really do anything as a return value. I put it there for clarity of all the global variables available to this class. – Cameron Barker Jul 30 '17 at 22:19

I find it weird that you wrote this as a module rather than as a class.

How is this code going to be called? Mail.new(user_id).send, I assume? It doesn't seem to make much sense to call #send twice on the same object. So, really, there isn't any point to make an object at all. You could just as well define a function.

def send_mail_to_user(user_id)
user = User.find(user_id)
client = user.mailchimp
campaign_id = client.create_campaign(user.detail.create_campaign_params)
client.create_campaign_content(campaign_id, user.detail.create_content_params)
client.send_campaign(campaign_id)
end

• Oh I made a mistake it should have a been a class, I've updated the code. Thanks for the feedback. I did imagine it being sent by Mail.new(user_id).send so that would make sense. In the example you gave, would you see that function fitting more into a module? – Cameron Barker Jul 29 '17 at 0:13

I would definitely go with a separate class or with a factory that solely represents the concern this campaign encloses.

On top of, I would assume (though not necessary), you could separate the mailchimp related implementation detail under a different class, which simply knows how to deal with it. Your actual code not suppose to know that low level detail.

However, it could be easily reachable through the campaign builder instance. What if you decide something else instead of Mailchimp ?, Just replace the instance with SomethingCampaignBuilder instance.

Based on those thoughts, I would rewrite to the following code -

class MailchimpCampaignBuilder

def initialize(user)
@user = user
end

def create_campaign(attrs)
mailchimp_client.create_campaign(attrs)
end

def update_campaign_content(campaign_id, attrs)
mailchimp_client
.create_campaign_content(campaign_id, attrs)
end

def deliver(campaign_id)
mailchimp_client.send_campaign(campaign_id)
end

def mailchimp_client
@mailchimp_client ||= user.mailchimp
end
end

class UserCampaignBuilder

CreateCampaignError = Class.new(StandardError)
UpdateCampaignError = Class.new(StandardError)

def self.deliver(user, campaign_builder)
builder = self.new(user, campaign_builder).build
builder.deliver
end

def initialize(user, campaign_builder)
@user             = user
@campaign_builder = campaign_builder
end

def build
initiate_new_campaign
.create_new_campaign
.update_campaign_content
end

def deliver
campaign_builder.deliver(campaign_id)
end

private

def initiate_new_campaign
@campaign_attributes          = user.detail.create_campaign_params
@campaign_content_attributes  = user.detail.create_content_params

self
end

def create_new_campaign
@campaign_id = campaign_builder.create_campaign(@campaign_attributes)

raise CreateCampaignError unless @campaign_id.present?

self
end

def update_campaign_content
updated = campaign_builder
.update_campaign_content(campaign_id, @campaign_content_attributes)

raise UpdateCampaignError unless updated

self
end
end

# Usags:
user              = User.find(134)
campaign_builder  = MailchimpCampaignBuilder.new(user)

UserCampaignBuilder.deliver(user, campaign_builder)