CODEWITHSUNDEEP
ruby-on-railsmodel-view-controller
Bob
Bob

Reputation: 8504

Where should I put this code?

My setup: Rails 2.3.10, Ruby 1.8.7

I need to update multiple instances of a model for a transaction. Should I make a class method and updates all the instances in the method or should I move that logic to the controller and updates each instance via an instance method for the model? I guess it is a tradeoff between fat controller vs. fat model and the general advice is fat model over fat controller.

Upvotes: 0

Views: 118

Answers (3)

dnch
dnch

Reputation: 9605

Neither. If it's a significant piece of logic, why not incorporate it into a dedicated class?

Alternatively, if your (I'm assuming) form data can be configured thus:

params[:models] = { id_of_instance_1 => { :attribute => value }, 
                    id_of_instance_2 => { :attribute => value2 },
                  }

You can quite easily do a group update in your controller with:

Model.update(params[:models].keys, params[:models].values)

More information about the details you're updating and where they're coming from could help.

EDIT: After reading your response below...

There's a few ways you could do it. You could implement Model.win and Model.lose as class methods to incorporate the logic, then simply call those methods from your controller:

def process_outcome
  @winner = Model.win(params[:winning_id])
  @loser = Model.lose(params[:losing_id])
end

Or, even as a single method call:

def process_outcome
  # passing the entire params hash to `process_outcome` which returns an array
  @winner, @loser = Model.process_outcome(params)
end

Personally, if the only child objects involved are all instances of the same model, I'd implement this logic within the class itself.

However, if you're bringing a variety of classes into the mix, it might be worth encapsulating it into a separate object altogether:

# app/controllers/models_controller.rb
def process_outcome
  @outcome_processor = OutcomeProcessor.new(params)
  @winner = @outcome_processor.winner
  @loser  = @outcome_processor.loser
end

Either way, your actual transaction block should not be in the Controller.

Upvotes: 2

Andrew Marshall
Andrew Marshall

Reputation: 96934

It should almost certainly go in the model, not the controller.

I think it should go in an instance method, not a class method. My reasoning behind this is that you're likely going to be calling this via the URL /model/id/action?other_model_id=other_id. Then it would follow that in the controller action you'd get appropriate model instance for id, as well as other_id, but since this is the path for the id model, not the other_id model, you'd call @id_model.perform_action(@other_id_model).

Hope this makes sense.

Upvotes: 0

Ashish
Ashish

Reputation: 5791

I think you should follow the tradition. :) You can use a different class (Not controllers) to write transaction method.

Upvotes: 0

Related Questions