pluginaweek / state_machine

Adds support for creating state machines for attributes on any Ruby class
http://www.pluginaweek.org
MIT License
3.74k stars 510 forks source link

delegating events to a separate object? #233

Closed dpickett closed 11 years ago

dpickett commented 11 years ago

I'd like to patch state_machine to include support for delegating event processing to an external object. Some state machines I've built out end up getting very long and end up violating the single responsibility principle. If I wrote something to handle this type of isolation, would it be merged in? Here's what I'm thinking:

  class Car
    event :park, Car::Events::ParkEvent
  end

  class CarEvents::ParkEvent < StateMachine::EventDelegate
    #transitions defined in a class method
    transition [:idling, :first_gear] => :parked

    def fire
      #this can optionally override the default behavior when car.park is called
    end
  end
obrie commented 11 years ago

This is an interesting question. For this case, I'd propose just using modules. For example:

module CarEvents
  module ParkEvent
    def self.extended(state_machine)
      state_machine.instance_eval do
        event :park do
          transition [:idling, :first_gear] => :parked
        end

        after_transition :on => :park, :do => :set_alarm
      end

      state_machine.owner_class.class_eval do
        include InstanceMethods
      end
    end

    module InstanceMethods
      def set_alarm
        puts "alarm set!"
      end
    end
  end
end

class Car
  state_machine :initial => :idling do
    extend CarEvents::ParkEvent
  end
end

car = Car.new
car.park

You could also just do an include from within the Car class, e.g.:

module CarEvents
  module ParkEvent
    def self.included(model)
      model.state_machine do
        event :park do
          transition [:idling, :first_gear] => :parked
        end

        after_transition :on => :park, :do => :set_alarm
      end
    end

    def set_alarm
      puts "alarm set!"
    end
  end
end

class Car
  state_machine :initial => :idling do
    # ...
  end

  include CarEvents::ParkEvent
end

car = Car.new
car.park
obrie commented 11 years ago

Going to close this one -- happy to re-open it if you have any feedback to do something other than the above proposed solution.

dpickett commented 11 years ago

Hey @obrie thanks for the quick response! This looks cool, but why would you prefer a module instead of a class? It seems like a lot more code for the user to write. Would you be opposed to a class based design?

the8472 commented 11 years ago

I'm also using modules to add "fragments" of statemachines into an existing one. But ruby's module API is slightly insufficient for that since we usually want to specify something like "add this cluster of states and transitions at point X into the process and have the final transition point to exit state Z", i.e. it needs to be parametrized.

obrie commented 11 years ago

@dpickett I think the alternative would be very complicated with little, if any, benefit over modules. You get the exact same API as in the model (don't need to change anything about how you're defining callbacks, event overrides, accessing the model, adding validations, etc.). You would basically end up inventing an entirely new API, complex backend to support all of the edge cases, and complicate instance-level logic, at the cost of 3-4 lines of code. Modules are pretty powerful and feel like the right approach in this case. You could even take the examples above and simplify them even further...

Suppose you didn't care about assuming what the model / machine was:


module ParkEvent
  def self.included(*)
    Car.state_machine do
      event :park do
        transition [:idling, :first_gear] => :parked
      end

      after_transition :on => :park, :do => :something
    end
  end

  def park
    super
    # do something else here
  end

  def something
    # do something
  end
end

class Car
  state_machine do
    # ...
  end
  include ParkEvent
end

Suppose you only wanted the transitions to be defined for an event:

module ParkEvent
  def self.definition
    lambda do
      transition [:idling, :first_gear] => :parked
    end
  end

  def park
    super
    # do something else here
  end
end

class Car
  state_machine do
    event(:park, &ParkEvent.definition)
  end
  include ParkEvent
end

Suppose you wanted to define transitions based on the state instead of the event:

module IdlingState
  def self.definition
    lambda do
      transition :to => :parked, :on => :park
    end
  end

  def park
    super
    # do something else here
  end
end

class Car
  state_machine do
    state(:idling, &IdlingState.definition)
  end
  include IdlingState
end

You could even imagine requiring files that make the appropriate modifications to the state machine:

car/park_event.rb:
Car.state_machine do
  event :park do
    transition [:idling, :first_gear] => :parked
  end
end

car.rb:
class Car
  state_machine do
    # ...
  end
end

require 'car/park_event'

There's probably a single recommended approach in there, but my personal feeling is that I don't think there's any advantage over the use of modules unless we were really bent on saving a few lines of code -- which doesn't seem worth it.

obrie commented 11 years ago

@the8472 Besides the likelihood that this is very much an edge case, there's no reason you couldn't parameterize using a tweak on one of the above solutions... e.g.:

module ParkEvent
  def self.definition(target)
    lambda do
      transition [:idling, :first_gear] => target
    end
  end

  def park
    super
    # do something else here
  end
end

class Car
  state_machine do
    event(:park, &ParkEvent.definition(:idling))
  end
  include ParkEvent
end

I think the overall idea is that there are a lot of ways you can take advantage of some of the features of Ruby to accomplish this without overcomplicating things.

dpickett commented 11 years ago

I understand your perspective and agree it would be a ton of work to do it right. I'll give the module solution you proposed a shot and see if it works for me! Thanks for being so thorough in your discussion of the issue, and for being an excellent maintainer of this gem.

obrie commented 11 years ago

@dpickett I actually had a conversation with @betamatt today about your idea and, after talking to him and getting his perspective, I understand a bit more the reason (from an object design perspective) for separating out the concerns into another class instead of mixing event-specific behaviors into the model via a module. Will follow up with more on this later...