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 D.’ 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