Template Method Pattern

Ruby on Rails
1

Hey Upcase,

I’d love to get some design pattern feedback here. I’m refactoring controller spaghetti in a Rails app that makes calls to an external service (Mailchimp). One step I’m taking in this refactor is extracting these API calls into separate classes, so the business logic of mailing list subscriptions is no longer in the controllers.

The app includes use cases for two different newsletter subscriptions, each of which collects an email address plus some additional (but distinct) user information. I’ve started by extracting a MailingListSubscription class, which is abstract and not used directly, and concrete subclasses for each of the two mailing lists.

Here’s the parent class:

# app/models/mailing_list_subscription.rb
class MailingListSubscription
  include ActiveModel::Model

  attr_accessor :email

  validates :email, presence: true

  def subscribe
    if valid?
      mailing_list_api.lists.subscribe(list_id, email_param, additional_params)
    end
  end

  # subclasses may override
  protected

  def list_id
    raise NotImplementedError, "Mailing list ID must be defined."
  end

  def additional_params
    nil
  end

  # subclasses should not override
  private

  def mailing_list_api
    @mailing_list_api ||= Mailchimp::API.new(ENV["MAILCHIMP_API_KEY"])
  end

  def email_param
    { email: email }
  end
end

And here’s an example child class:

# app/models/weekly_news_subscription.rb
class WeeklyNewsSubscription < MailingListSubscription
  attr_accessor :first_name, :last_name

  validates :first_name, :last_name, presence: true

  private

  def list_id
    ENV["MAILCHIMP_WEEKLY_NEWS_LIST_ID"]
  end

  def additional_params
    {
      "FNAME" => first_name,
      "LNAME" => last_name,
    }
  end
end

The other mailing list will have a different list_id and a separate set of additional_params.

Overall, I’m pleased with these changes compared to the current controller code.

I do have two questions I’m considering:

  1. Is this this design pattern the most appropriate way to structure these models? Is there another pattern (perhaps using composition instead of inheritance) that might work better?

  2. What is the best way to unit test these models? (I’d like to test the parent class, if possible, but since it depends on the implementation of list_id provided by a subclass, I can’t test MailingListSubscription#subscribe in isolation. I can unit test the subclasses, but they don’t directly implement subscribe, and that leads to some duplication in the tests.)

Thanks for your thoughts!

2

You could avoid using inheritance and inject the subscription? Something like (I haven’t ran this):

class SubscriptionManager # there's probably a better name for this
  include ActiveModel::Model

  attr_accessor :email
  attr_accessor :list

  validates :email, presence: true

  def subscribe
    if valid?
      mailing_list_api.lists.subscribe(@list.id, email_param, @list.additional_params)
    end
  end

  private

  def mailing_list_api
    @mailing_list_api ||= Mailchimp::API.new(ENV["MAILCHIMP_API_KEY"])
  end

  def email_param
    { email: email }
  end
end

class WeeklyNewsSubscription
  attr_accessor :first_name, :last_name

  validates :first_name, :last_name, presence: true

  def list_id
    ENV["MAILCHIMP_WEEKLY_NEWS_LIST_ID"]
  end

  def additional_params
    {
      "FNAME" => first_name,
      "LNAME" => last_name,
    }
  end
end

weekly_news_subscription = WeeklyNewsSubscription.new(first_name: "...", last_name: "...")
SubscriptionManager.new(email: "andy@example.com", list: weekly_news_subscription).subscribe

You can then test the manager class using a double, and test each subscription class in isolation.

BTW, how do you enable the color syntax highlighting?

3

Naming classes Manager makes a class an attractor for lots of different responsibilities. I’d probably name this something like SubscribeMailingList (making it more of a service object) and name the method call, or keep the an alias to #subscribe.

1 Like
4

You could avoid using inheritance and inject the subscription

Thanks, Andy; I think this is what I was looking for. I’ll give this a shot.

BTW, how do you enable the color syntax highlighting?

How do you format your code blocks? Apparently Discourse auto-highlights fenced code blocks (delimited with 3 backticks), but not code that’s indented four spaces.