extracting a portion of a model to a separate file

G'day,

I have a project that relies heavily on pluginaweeks "state_machine" gem. While I would have liked to post this to a state_machine forum, 1. I couldn't find one and 2. this is probably a generic ruby concept that I just can't piece together on my own - so asking here may be *more* appropriate (but still less appropriate than a straight ruby forum... maybe).

I have a ton of models that each have fairly complex state machines. state_machine defines them with blocks directly in your model class like so:

state_machine :initial => :created do   state :created

  event :review do     transition any => :reviewing   end end

My state_machine blocks however are very large and clutter up my model.rb files...

What I want to do is extract out all of the state machine code to another file that I can then "include" in my model. I know include is the wrong terminology in ruby.. maybe I want to modularize them?

Whatever it's called. It's preferable (for maintainability, etc) for my models to look something like

class Thing < ActiveRecord::Base   #validations   #callbacks   #etc...

  line_of_code_that_loads_external_state_file_here   #maybe eval somehow   #maybe make a module that gets "mixed in"... i really dont know the right approach end

Any help would be appreciated. - FJM

frankjmattia@gmail.com wrote:

G'day,

I have a project that relies heavily on pluginaweeks "state_machine" gem. While I would have liked to post this to a state_machine forum, 1. I couldn't find one and 2. this is probably a generic ruby concept that I just can't piece together on my own - so asking here may be *more* appropriate (but still less appropriate than a straight ruby forum... maybe).

I have a ton of models that each have fairly complex state machines. state_machine defines them with blocks directly in your model class like so:

state_machine :initial => :created do   state :created

  event :review do     transition any => :reviewing   end end

My state_machine blocks however are very large and clutter up my model.rb files...

IMHO, they don't *clutter* your models; they are *part of* your models. I tend to think you want the state_machine blocks in the model files so that you can review your state machine behaviors right along with your methods and everything else. So I'm not sure that it's such a great idea to do what you're asking.

However, it's not that hard. Read on for ideas!

What I want to do is extract out all of the state machine code to another file that I can then "include" in my model. I know include is the wrong terminology in ruby.. maybe I want to modularize them?

Whatever it's called. It's preferable (for maintainability, etc)

Why would this help maintainability? It seems to me that it would be more maintainable to keep it all together as outlined above?

for my models to look something like

class Thing < ActiveRecord::Base   #validations   #callbacks   #etc...

  line_of_code_that_loads_external_state_file_here   #maybe eval somehow   #maybe make a module that gets "mixed in"... i really dont know the right approach end

Well, you could take several approaches here. You could reopen the class in a separate file... ### app/models/thing.rb class Thing < ActiveRecord::Base   # validations   # methods   require 'thing_states' end

### lib/thing_states.rb class Thing < ActiveRecord::Base   state_machine stuff end

...you could store the state machine configuration in a YAML file or something and read it with YAML.load or whatever that function is...you could write a wrapper function for state_machine with a shorter syntax...

But again, I think that this is probably not a great idea. If your state machine definitions are part of your model functionality, they should probably be in the same model file with everything else. If your state machines are just "clutter", then they probably shouldn't be in your code at all. (Note all the "probably"s here -- I could well be wrong.)

Any help would be appreciated. - FJM

Best,

> G'day,

> I have a project that relies heavily on pluginaweeks "state_machine" > gem. While I would have liked to post this to a state_machine forum, > 1. I couldn't find one and 2. this is probably a generic ruby concept > that I just can't piece together on my own - so asking here may be > *more* appropriate (but still less appropriate than a straight ruby > forum... maybe).

> I have a ton of models that each have fairly complex state machines. > state_machine defines them with blocks directly in your model class > like so:

> state_machine :initial => :created do > state :created

> event :review do > transition any => :reviewing > end > end

> My state_machine blocks however are very large and clutter up my > model.rb files...

IMHO, they don't *clutter* your models; they are *part of* your models. I tend to think you want the state_machine blocks in the model files so that you can review your state machine behaviors right along with your methods and everything else. So I'm not sure that it's such a great idea to do what you're asking.

However, it's not that hard. Read on for ideas!

> What I want to do is extract out all of the state machine code to > another file that I can then "include" in my model. I know include is > the wrong terminology in ruby.. maybe I want to modularize them?

> Whatever it's called. It's preferable (for maintainability, etc)

Why would this help maintainability? It seems to me that it would be more maintainable to keep it all together as outlined above?

Maintainability was probably not the best word. Readability? Looking for a better abstraction? I dunno. Having a model with only 300 lines of code with 80 of those for a state machine -- looks ugly. There has to be a more readable abstraction.

Now that I see how it works, I'm thinking something like:

instance_eval(File.read("states.rb"), "states.rb")

> for > my models to look something like

> class Thing < ActiveRecord::Base > #validations > #callbacks > #etc...

> line_of_code_that_loads_external_state_file_here > #maybe eval somehow > #maybe make a module that gets "mixed in"... i really dont know the > right approach > end

Well, you could take several approaches here. You could reopen the class in a separate file... ### app/models/thing.rb class Thing < ActiveRecord::Base # validations # methods require 'thing_states' end

### lib/thing_states.rb class Thing < ActiveRecord::Base state_machine stuff end

...you could store the state machine configuration in a YAML file or something and read it with YAML.load or whatever that function is...you could write a wrapper function for state_machine with a shorter syntax...

But again, I think that this is probably not a great idea. If your state machine definitions are part of your model functionality, they should probably be in the same model file with everything else. If your state machines are just "clutter", then they probably shouldn't be in your code at all. (Note all the "probably"s here -- I could well be wrong.)

They are definitely not "clutter" I just meant that they clutter up an otherwise readable file. I think making some sort of wrapper for instance_eval that figures out the model name then instance evals it from say... app/models/states/order_states.rb or somesuch.

I see your point about keeping it together but there comes a point where an abstraction becomes necessary... I may not be at that exact point yet, but I see it over the horizon and want to work it out now.

frankjmattia@gmail.com wrote:

> I have a ton of models that each have fairly complex state machines.

> What I want to do is extract out all of the state machine code to > another file that I can then "include" in my model. I know include is > the wrong terminology in ruby.. maybe I want to modularize them?

> Whatever it's called. It's preferable (for maintainability, etc)

Why would this help maintainability? �It seems to me that it would be more maintainable to keep it all together as outlined above?

Maintainability was probably not the best word. Readability? Looking for a better abstraction? I dunno. Having a model with only 300 lines of code with 80 of those for a state machine -- looks ugly. There has to be a more readable abstraction.

Then I'd recommend finding a more readable abstraction that keeps it all in the same place. Moving your state machine code to a separate file essentially means that an important piece of your model functionality is invisible when you most need to see it.

Now that I see how it works, I'm thinking something like:

instance_eval(File.read("states.rb"), "states.rb")

I think you've got the instance_eval syntax wrong here, but why on earth would you use instance_eval instead of require or include?

There are legitimate uses for eval and instance_eval in Ruby, but they are few and far between -- enough so that their use should IMHO be considered a code smell. If you think you need eval or instance_eval, stop and think -- there's almost always a better way.

[...]

I see your point about keeping it together but there comes a point where an abstraction becomes necessary...

What are you trying to abstract? Moving to a separate file is not, in itself, abstraction. If an abstraction is what's wanted here, let's find one instead of shuffling the same code around between files.

I may not be at that exact point yet, but I see it over the horizon and want to work it out now.

Nothing wrong with speculating about future directions; just beware of premature optimization.

Best,

Marnen Laibow-Koser wrote:

> I have a ton of models that each have fairly complex state machines.

> What I want to do is extract out all of the state machine code to > another file that I can then "include" in my model. I know include is > the wrong terminology in ruby.. maybe I want to modularize them?

> Whatever it's called. It's preferable (for maintainability, etc)

Why would this help maintainability? �It seems to me that it would be more maintainable to keep it all together as outlined above?

Maintainability was probably not the best word. Readability? Looking for a better abstraction? I dunno. Having a model with only 300 lines of code with 80 of those for a state machine -- looks ugly. There has to be a more readable abstraction.

Then I'd recommend finding a more readable abstraction that keeps it all in the same place. Moving your state machine code to a separate file essentially means that an important piece of your model functionality is invisible when you most need to see it.

Now that I see how it works, I'm thinking something like:

instance_eval(File.read("states.rb"), "states.rb")

I think you've got the instance_eval syntax wrong here, but why on earth would you use instance_eval instead of require or include?

Because neither require nor include do what I need them to do. Require runs another file but if that other file relies on functions defined on the requiring class, it can't access them... can it? And include takes all the methods in the included file and adds them to the including class. Except the state definitions aren't method definitions. They are class functions that accept blocks which need to be run in the context of the calling class. (Please forgive me if my terminology isn't entirely accurate. I'm still asking why class_eval and instance_eval were named they way they were.)

There are legitimate uses for eval and instance_eval in Ruby, but they are few and far between -- enough so that their use should IMHO be considered a code smell. If you think you need eval or instance_eval, stop and think -- there's almost always a better way.

[...]

I see your point about keeping it together but there comes a point where an abstraction becomes necessary...

What are you trying to abstract? Moving to a separate file is not, in itself, abstraction. If an abstraction is what's wanted here, let's find one instead of shuffling the same code around between files.

Yeah, this is more of a "code shuffling" today. Eventually I'd like to encapsulate state definitions into a reusable chunk of code so that multiple models can rely on the same definitions without the brittleness that comes with repeating them. A wrapper around instance_eval lets me reuse state_machines if I so desire without having to duplicate their code.

Frank Mattia wrote: [...]

Now that I see how it works, I'm thinking something like:

instance_eval(File.read("states.rb"), "states.rb")

I think you've got the instance_eval syntax wrong here, but why on earth would you use instance_eval instead of require or include?

Because neither require nor include do what I need them to do. Require runs another file but if that other file relies on functions defined on the requiring class, it can't access them... can it?

Sure. Just reopen the requiring class. This is exactly what I was doing in my example in Extracting a portion of a model to a separate file - Rails - Ruby-Forum ; you may want to review it.

And include takes all the methods in the included file and adds them to the including class. Except the state definitions aren't method definitions. They are class functions that accept blocks which need to be run in the context of the calling class.

Good point. You'll probably need extend instead of include, and you may need the included and/or extended hooks. But this is definitely possible.

[...]

What are you trying to abstract? Moving to a separate file is not, in itself, abstraction. If an abstraction is what's wanted here, let's find one instead of shuffling the same code around between files.

Yeah, this is more of a "code shuffling" today. Eventually I'd like to encapsulate state definitions into a reusable chunk of code so that multiple models can rely on the same definitions without the brittleness that comes with repeating them.

Then do it right and put it into a module (or use inheritance).

A wrapper around instance_eval lets me reuse state_machines if I so desire without having to duplicate their code.

Nope! instance_eval just introduces brittleness. Learn about Ruby's module system and use that instead.

You almost never need to evaluate arbitrary literal code in Ruby, which means you almost never need *eval. Metaprogramming is an amazing thing.

Best,

Gentlemen thanks for having this conversation. Was pulling my hair out trying to get state_machine to work with a mixin. I need to define state_machines behaviors unique to each client while keeping a single code base. I ended up using opening up the class like Maren suggested.

class OnDemandWorkorderWorkflow < Workflow

  belongs_to :workorder

  attr_accessible :type, :state, :workorder_id

  require "#{Rails.root}/lib/clients/#{CUSTOMER}/workflows/#{FILE_REFERENCE}_on_demand_workorder_workflow"

end

required file looks like this:

class OnDemandWorkorderWorkflow < Workflow

  state_machine :state, :initial => :new do     .......     # lots of events and transitions     ....... end

One mechanism that can be helpful that hasn’t been mentioned - ActiveSupport::Concern. See here for more detail:

http://www.fakingfantastic.com/2010/09/20/concerning-yourself-with-active-support-concern/

–Matt Jones

Matt Jones wrote in post #1080951:

I have a ton of models that each have fairly complex state machines.

My state_machine blocks however are very large and clutter up my model.rb files...

What I want to do is extract out all of the state machine code to another file that I can then "include" in my model. I know include is the wrong terminology in ruby.. maybe I want to modularize them?

One mechanism that can be helpful that hasn't been mentioned - ActiveSupport::Concern. See here for more detail:

Funny, as soon as I came back to check the update I said to myself "knowing what I know now, I would have probably used a concern". I second your suggestion.

http://www.fakingfantastic.com/2010/09/20/concerning-yourself-with-active-support-concern/