Trying to understand the point of Gary Bernhardt's "Boundaries" talk

original.rb

describe Sweeper do
  context 'a subscription is expired' do
    let(:bob) { double(active?: true, paid_at: 2.months.ago) }
    let(:users) { [bob] }
    before { allow(User).to receive(:all).and_return(users) }
    
    it 'emails the user' do
      expect(UserMailer).to receive(:billing_problem).with(bob)
      described_class.sweep
    end
  end
end

class Sweeper
  def self.sweep
    User.all.select do |user|
      user.active? and user.paid_at < 1.month.ago
    end.each do |user|
      UserMailer.billing_problem(bob)
    end
  end
end

refactored.rb

describe Sweeper do
  context 'a subscription is expired' do
    let(:bob) { double(active?: true, paid_at: 2.months.ago) }
    
    it 'emails the user' do
      expect(ExpiredUsers.for_users(users)).to eq [bob]
    end
  end
end

class ExpiredUsers
  def self.for_users(user)
    users.select do |user|
      user.active? and user.paid_at < 1.month.ago
    end
  end
end

class Sweeper
  def self.sweep
    ExpiredUsers.for_users(User.all).each do |user|
      UserMailer.billing_problem(user)
    end
  end
end

You could do the integration test of Sweeper.sweep with e.g. three users

This argument doesn't scale up with the combinatoric explosion of different branches as code complexity increases.

@xaviershay Iiiinteresting point about not testing it at all because the test would mirror the implementation. This reminds me of the argument against testing e.g. "has_many :users" or "should have_db_column(:foo)" - this is not code that you would ever refactor, hence tests accomplish very little. Thanks!

I think the trouble here is that the example Gary created is somewhat contrived. Where I think it becomes very obviously useful is when you have complex conditional logic mixed with imperative logic that modifies state.

At that point, when you factor our the functional stuff, it's all really easy to test because you just assert on values. The imperative stuff is easy to test because you only need one good test that shows that the functional innards got called. Whether you decide to do that with a stub and some kind of rspec-fire/bogus thing that ensures the contract, or an actual integration test (my preference in most cases) - you assume that the functional innards can do their job well, so you only have 1 happy path test on the integration side.

I mentioned over twitter that we recently refactored an accounting system in this way. When we started refactoring we didn't think about Boundaries or functional vs imperative, we just started separating things by SRP principle. The result incidentally became a split between the functional "Line Item Builder" which was concerned with lots of math and conditionals (things like calculating fees, etc), and the imperative "Statement Builder" which wrote line items to the database as part of a Statement object.

The original code had conflated it all into one object called a Statement Builder. The only way to meaningfully test it, since it wrote stuff to the database, was to have N paths for every kind of line item it was creating and to assert on database state after the fact.

Post our refactoring, we have N tests in the functional line item piece, covering all the conditional and math logic. It does not talk to the DB. On the imperative StatmentBuilder side we basically have 1 test that says given a line item, you attach it to a statement. There's a pretty big win in terms of test speed here, as well as test readability. But I think most of it came from just applying SRP principles. The functional/imperative split was a nice cherry on top. After I realized we had actually done the functional/imperative split - I said - hey this is Gary's boundaries at work. And now I think this technique will help me look more for code that doesn't belong together.

My accounting stuff is too Reverb specific to post, but at some point in the future if I have a good example of this type of refactoring, I'll make a post on the Reverb Dev Blog