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
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
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.
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?
Once you correct the code, it won’t cause any more exceptions. 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.
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.