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?

5
  • 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 '17 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 '17 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 '17 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 '17 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 at 20:59
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

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.