Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Refactor FSM#transition #99

Merged
merged 6 commits into from

2 participants

@JoeyButler

Noticed that this method was pretty gnarly at a flog rating of 60.15. This branch improves the test coverage slightly and greatly improves the form of the method. After applying a few different refactoring techniques I was able to get it down to a rating of 6.

As always you could go a bit further with cleaning it up, but I thought that this was a decent size branch as is.

Feel free to provide constructive criticism, or mention any stylistic preferences!

@tarcieri tarcieri merged commit 46bb562 into celluloid:master
@tarcieri
Owner

Thanks! This is awesome stuff. Makes me feel Celluloid::FSM is slightly less ghetto ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 4, 2012
  1. Add test around FSM#transition

    Joey Butler authored
  2. Extra method refactoring

    Joey Butler authored
  3. Improve test coverage for FSM#transition

    Joey Butler authored
  4. Extract out FSM state methods

    Joey Butler authored
  5. The final extract method refactoring that tames the beast formerly known

    Joey Butler authored
    as FSM#transition
This page is out of date. Refresh to see the latest.
Showing with 73 additions and 16 deletions.
  1. +49 −16 lib/celluloid/fsm.rb
  2. +24 −0 spec/celluloid/fsm_spec.rb
View
65 lib/celluloid/fsm.rb
@@ -84,30 +84,71 @@ def attach(actor)
#
# Note: making additional state transitions will cancel delayed transitions
def transition(state_name, options = {})
+ new_state = validate_and_sanitize_new_state(state_name)
+ return unless new_state
+
+ if handle_delayed_transitions(new_state, options[:delay])
+ return @delayed_transition
+ end
+
+ transition_with_callbacks!(new_state)
+ end
+
+ # Immediate state transition with no sanity checks, or callbacks. "Dangerous!"
+ def transition!(state_name)
+ @state = state_name
+ end
+
+ protected
+
+ def validate_and_sanitize_new_state(state_name)
state_name = state_name.to_sym
- current_state = self.class.states[@state]
- return if current_state && current_state.name == state_name
+ return if current_state_name == state_name
if current_state and not current_state.valid_transition? state_name
valid = current_state.transitions.map(&:to_s).join(", ")
raise ArgumentError, "#{self.class} can't change state from '#{@state}' to '#{state_name}', only to: #{valid}"
end
- new_state = self.class.states[state_name]
+ new_state = states[state_name]
unless new_state
- return if state_name == self.class.default_state
+ return if state_name == default_state
raise ArgumentError, "invalid state for #{self.class}: #{state_name}"
end
- if options[:delay]
+ new_state
+ end
+
+ def transition_with_callbacks!(state_name)
+ transition! state_name.name
+ state_name.call(self)
+ end
+
+ def states
+ self.class.states
+ end
+
+ def default_state
+ self.class.default_state
+ end
+
+ def current_state
+ states[@state]
+ end
+
+ def current_state_name
+ current_state && current_state.name || ''
+ end
+
+ def handle_delayed_transitions(new_state, delay)
+ if delay
raise UnattachedError, "can't delay unless attached" unless @actor
@delayed_transition.cancel if @delayed_transition
- @delayed_transition = @actor.after(options[:delay]) do
- transition! new_state.name
- new_state.call(self)
+ @delayed_transition = @actor.after(delay) do
+ transition_with_callbacks!(new_state)
end
return @delayed_transition
@@ -117,14 +158,6 @@ def transition(state_name, options = {})
@delayed_transition.cancel
@delayed_transition = nil
end
-
- transition! new_state.name
- new_state.call(self)
- end
-
- # Immediate state transition with no sanity checks. "Dangerous!"
- def transition!(state_name)
- @state = state_name
end
# FSM states as declared by Celluloid::FSM.state
View
24 spec/celluloid/fsm_spec.rb
@@ -83,4 +83,28 @@ class CustomDefaultMachine
subject.state.should == :pre_done
end
+
+ context "actor is not set" do
+ let(:subject) { TestMachine.new }
+
+ context "transition is delayed" do
+ it "raises an unattached error" do
+ expect { subject.transition :another, :delay => 100 } \
+ .to raise_error(Celluloid::FSM::UnattachedError)
+ end
+ end
+ end
+
+ context "transitioning to an invalid state" do
+ let(:subject) { TestMachine.new }
+
+ it "raises an argument error" do
+ expect { subject.transition :invalid_state }.to raise_error(ArgumentError)
+ end
+
+ it "should not call transition! if the state is :default" do
+ subject.should_not_receive :transition!
+ subject.transition :default
+ end
+ end
end
Something went wrong with that request. Please try again.