How to better factor classes that switch on type code

Question

We have a persistence backed object, and it has a version field in the database. Assume there's lots of unrelated code in this class.

#
# We're branching on this "version" field, and we return one
# of 2 policies.
#
class Foo
  def policy
    if version == 'a'
      PolicyA.new
    else
      PolicyB.new
    end
  end
end

#
# PolicyA calls a single check on a collaborator.
#
class PolicyA
  def pass?(collaborator)
    collaborator.check_x?
  end
end

#
# PolicyB calls two different checks.
#
class PolicyB
  def pass?(collaborator)
    collaborator.check_x? && collaborator.check_y?
  end
end

#
# One day, we need a 3rd policy version.
#
class Foo
  def policy
    if version == 'a'
      PolicyA.new
    elsif version == 'b'
      PolicyB.new
    else
      PolicyC.new
    end
  end
end

#
# PolicyC needs 2 collaborators, so we add a second argument
# and assign a default.
#
class PolicyA
  def pass?(collaborator_a, _collaborator_b = nil)
    collaborator_a.check_x?
  end
end

#
# Same here.
#
class PolicyB
  def pass?(collaborator_a, _collaborator_b = nil)
    collaborator_a.check_x? && collaborator_a.check_y?
  end
end

#
# Here we pass in a second collaborator and query it.
#
class PolicyC
  def pass?(collaborator_a, collaborator_b)
    collaborator_a.check_y? && collaborator_b.check_z?
  end
end

#
# Some code has to use this thing.
#
class PolicyChecker
  def check(foo)
    foo.policy.pass?(collaborator_a, collaborator_b)
  end
end

Is there a good way out of this that doesn't involve subclassing Foo?

What about dealing with these collaborators? What happens when a third one comes along?

Reaching into foo to get at policy feels wrong, but policy knowing about foo.version also feels wrong.

Whats a good path forward from here?

Answer 1

The Foo#policy method seems ok.

I think the main problem is PolicyX#pass? method; at the moment it is exposing too much. To use it, you have to know about the collaborators of the PolicyX class.

A good solution could be to inject collaborators when you instantiate the object. Lets make an example with PolicyC

class PolicyC
  def initialize(collaborator_a,collaborator_b)
    @collaborator_a = collaborator_a
    @collaborator_b = collaborator_b
  end
  def pass?
    @collaborator_a.check? && collaborator_b.check? # 
  end
end

Answer 2

The type code is a red herring.

Your situation is that you have three policies A, B and C. A and B are implemented differently, but can be abstracted using the same principle: The caller passes a collaborator, and the policy returns whether the collaborator passes or not. The caller can then proceed with the “pass” information alone and doesn’t care which policy produced it and how.

C is very different: it needs two collaborators presumably to determine if the first one passes. So the caller has to know this, they have to call C differently, and they need to have a second collaborator and what it’s there for - I’d assume this has to be a particular person.

So where do we go from there: It doesn’t fit nicely at all. I’d say the caller just has to write some considerable code to handle the cases. Either by using a switch statement, or by subclassing, or by storing a pointer to code performing the right action.