0

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?

6
  • Is the needed implementation of PolicyA dependent only on version == 'a' or does it need to be version "a" of Foo? If I had a Bar that was version "a" could I reuse any of this code? Mar 25, 2017 at 3:36
  • Its only dependant on version == 'a'. You would not have a 'Bar' with any of Foo's version codes.
    – Rorshark
    Mar 25, 2017 at 15:50
  • If the policy is only dependent on 'a' and not Foo then remove the policy code from Foo and make the policies into services that Foo or whatever uses thru a policy interface. Mar 25, 2017 at 16:11
  • 1
    It feels like a "factory" used at the data mapping layer would be a good solution here, so the class doesn't make the decision. Ultimately, somebody has to decide this. Mar 25, 2017 at 18:06
  • Any reason you have to create a new policy class every time? What about simply having the policy class as an instance variable. If you need to change it you change it, but there is no visible reason why it needs to be created new when you can simply reference it. Jul 31, 2021 at 20:59

2 Answers 2

0

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

0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.