Moving code to model from controller

Hi
   Last day I read an article says that business logic should be in
model Till now my coding method was writing complete code in controller
and nothing in model I would like to change that style. Sorry now only
I became aware of that, since I am not an expert programmer and also and
of self learning process. My current code in controller is as below
This for creating a problem

def create
    begin
      ActiveRecord::Base.transaction do
        @problem=Problem.new(params[:problem])
        @problem.save!
        @problem.update_attribute("number","PB"+ @problem.id.to_s)
      end #end of transaction
      rescue ActiveRecord::ActiveRecordError => e:
        puts 'Error' +e.to_s
        render :action => 'create_ui'
      else
        redirect_to(:action => 'show_problem_ui', :id => @problem.id)
    end #end of begin
end

       Could anybody please tell me how can I shift the above code to
Problem model and follow the proper MVC pattern .The above code is
minimal but in almost all parts of the code I have followed the same
pattern ie write complete code in controller And if I get any good rails
programming practises links that will be helpful

Thanks in advance
Sijo

In this case, the only thing that the controller is doing that the model could/should is setting the problem number. Since the problem number should be set any time a problem is created, move that logic into an after_create callback inside the model.

class Problem < ActiveRecord::Base

after_create :set_problem_number

private

def set_problem_number
self.number = “PB#{self.id}”

save!

end
end

I’d use a callback in the model instead of using an observer in this case because setting the problem number is work done to the model. If, for example, we wanted to log a message or send a notification of some kind, I’d use an observer, since that kind of work is not done to the model.

Whether to move logic to the model is a question you have to ask in each case like the one you shared. Sometimes it’s best to move code to a place other than the model, view or controller. See the Rails Refactoring Catalog [ http://continuousthinking.com/2008/6/3/refactoring-your-rails-application-tutorial-content ] from Zach Dennis’ talk at RailsConf 2008 for more details.

Regards,
Craig

Hi
  Thanks for your reply
Sijo

Hi
   I did as you said ie set a callback after_create as below

def set_problem_number
    begin
      Problem.transaction do
        self.number = "PB#{self.i}"
        save!
      end # end of transaction
       rescue Exception => e:
       puts 'Error' +e.to_s
    end # end of begin-end
end

    In the above to test to get an exception I deliberately wrote
  self.number = "PB#{self.i}" instead of self.number = "PB#{self.id}"
So what happens is a new problem record row is created in table but how
ever its number say for exapme PB77 is not set ..So how can I roll back
that to if any exception happened in the above call back in model?

Sijo

Once you correct the code, it won’t cause any more exceptions. :slight_smile: Also, since you’re using the after_create callback, your code is running after the Problem record has been created. Hence, there’s nothing you can do to prevent the Problem record from being created.

An additional note: the transaction in the callback is unnecessary. I don’t think that it runs in the scope of the creation of the problem record that you’re doing in the controller, and you’re working with only one object and database operation in the callback anyway.

Regards,
Craig