separate state and event callbacks

[kjwierenga]

In my opinion the on_enter and on_exit callbacks for events are confusing.

States and events are different concepts. By allowing on_enter and on_exit on events a single namespace is created for states and events leading to ambiguous code. I.e. when you have an event :digit and a state :digit what does the callback on_enter_digit mean?

I propose to have separate before and after callbacks for events:

callbacks {
  before_digit  { |event, digit| digits << digit }
  after_on_hook { |event|        disconnect! }
}

What was your rationale for merging callbacks for states and events?

[piotrmurach]

Agree, the on_enter, on_exit, on_transition would serve well in the context of states and on_before and on_after would feel better in describing event callbacks.

My rational was simply to provide uniform interface and let the code sweat out what the user means, whether on_enter refers to state or event. However, as you have rightly pointed out this can lead to confusion when state and event are named the same.

[piotrmurach]

This is implemented in the master as agreed, I've also updated docs

[kjwierenga]

Nice. So now I have the following:

  callbacks {
    on_exit_state       { |event| puts "exit_#{event.from}" }
    on_before_event     { |event| puts "\tbefore_#{event.name}"    }
    on_transition_state { |event| puts "\t\ttransition: #{event.name}: #{event.from} -> #{event.to}" }
    on_enter_state      { |event| puts "\tenter_#{event.to}"  }
    on_after_event      { |event| puts "after_#{event.name}"     }
  }

Since events and states are now independent (from callback perspective), I guess you could drop the trailing _event and _state. This would be the same as an implicit :any argument which would mean any event or any state dependent on the context. Could that work?

  callbacks {
    on_exit       { |event| puts "exit_#{event.from}" }
    on_before     { |event| puts "\tbefore_#{event.name}"    }
    on_transition { |event| puts "\t\ttransition: #{event.name}: #{event.from} -> #{event.to}" }
    on_enter      { |event| puts "\tenter_#{event.to}"  }
    on_after      { |event| puts "after_#{event.name}"     }
  }

Mind you if I try that, I get the following InvalidCallbackNameError exception, which indicates that internally there is still some confusion about states and events (on_exit argument can only be state but exception mentions 'any' event name).

"on_exit" callback is a state listener and cannot be used with "any" event name. Please use on_before or on_after instead.

[piotrmurach]

Very good point - working on it. I've added some validations yesterday to throw InvalidCallbackNameError for when state callback is used with event name etc.... This kind of works now 😄

[kjwierenga]

Seems your callback error checking is a bit too strict now. Because state and event namespace is now separate, you should be able to have a state and an event with the same name.

The following state machine is perfectly reasonable and works when the callbacks are removed.

require 'finite_machine'

phone = FiniteMachine.define do
  initial :on_hook

  events {
    event :off_hook, :on_hook  => :off_hook
    event :on_hook,  :off_hook => :on_hook
  }

  callbacks {
    on_before(:on_hook) { puts "receive on_hook event"  }
    on_enter(:on_hook)  { puts "entering on_hook state" }
  }
end

phone.off_hook
phone.on_hook

Event names and state names should be allowed to be the same I think.

[piotrmurach]

Fixed on master!

[kjwierenga]

Thanks.