What is a good way of satisfying open/closed principle in this case

Question

I have a class that looks like this:

class MyClass

  # takes an array of actions
  def initialize(actions)
    @actions = actions 
  end

  def act
    @actions.each {|a| a.do}
  end 
end

class Action 

  attr_reader :type
  def initialize(func, type)
    @type = type
    @func = func
  end

  def do
    @func.call
  end
end

So far pretty simple. However. Depending on the type, the actions have to be called in different order. E.g., all the actions with the type "start" have to be called before all the actions with the type "finish". The question is, where should this logic live?

The obvious answer to me was, in the act method of MyClass. But this suddenly looks very awkward:

  def act
    @actions.select {|a| a.type == "start"}.each {|a| a.do}
    @actions.select {|a| a.type == "finish"}.each {|a| a.do}
  end 

Moreover this is against open/closed principle as I understand it, since if there are new types I will have to modify the code in the act method.

What are some other options I have?

Answer 1

In what way is this interface closed for modification but open for extension?

What if "middle" needs to be executed between "start" and "finish"? You have to modify Action.

What if instead of MyClass we passed the Action objects to TheirClass? Either TheirClass completely mucks up the order of execution a bug now, or MyClass and TheirClass are subtly coupled - a bug waiting for the next developer to change one, and forget the other.

A Design Problem

The defect in your solution is that a set of Action objects is lazily expecting whomever receives them to be an expert in how they should be scheduled.

This is like assembling flat pack furniture. Sure it is possible to distribute assembly and you generally get the same result, but someone out there will have screws left over and no clue where they belong... Computers are really fast at this so they generally discover a lot of those cases, we call them bugs (or defects).

Pass the Action as intended instead

So the Action set should take back that power of scheduling. This way it will happen exactly as desired.

Forgive my ruby, It's not a language I generally use:

class MyClass

  # takes an array of actions
  def initialize(action)
    @action = action
  end

  def act
    @action.do
  end 
end

class Action 

  attr_reader :type
  def initialize(func)
    @func = func
  end

  def do
    @func.call
  end
end

class StartFinishAction 

  attr_reader :type
  def initialize(start_actions, finish_actions)
    @start_actions= start_actions
    @finish_actions= finish_actions
  end

  def add_start(func)
    @start_actions << func
  end
  def add_finish(func)
    @finish_actions << func
  end

  def do
    @start_actions.each {|a| a.do}
    @finish_actions.each {|a| a.do}
  end
end

This is a much better implementation.

• Need to add a conditional action? Then create a ConditionalAction class with a do method, neither MyClass nor TheirClass will care.
• Need to add a "middle" set of actions? Then either extend the current class, or implement a new one.
• Don't need Start and Finish actions? Then just pass a normal Action in, or whatever other Actionesque classes you may desire instead.

This is closed for modification, but open for extension.

Lambdas

Congratulations you have just reinvented lambdas. I believe Ruby loves them.

Lambdas are great in that they are code, which is exactly the use case for an Action. Its a perfect fit. Let the language itself deal with the complexities of code, while you get the benefit of having the entire development environment support writing and debugging.

The boiler plate heavy approach above, can be used to implement lambdas in a language which does not support them. You might still take this approach if for some reason the Action needs to be more than just code (for execution) but also data (for optimisation, reflection, persistence, ...). Generally this is the case for interpreters and compilers.