Beginners question | moving a method from controller to model

rubyonrails-talk
1

I'm learning with the Agile Web Development 4th edition book. I've built a method in my controller to test that it works, and now I want to move it to the model.

2

I’m learning with the Agile Web Development 4th edition book. I’ve

built a method in my controller to test that it works, and now I want to

move it to the model.

*** line_item controller ***

def create

@cart = current_cart

 event = Event.find(params[:event_id])

 @line_item = @cart.add_event(event)

  new_capacity = event.capacity





respond_to do |format|

  if @line_item.save

    new_capacity =  new_capacity - @line_item.quantity

    event.capacity = new_capacity

    event.save



    format.html { redirect_to(@line_item.cart, :notice => 'Line item

was successfully created.') }

    format.xml  { render :xml => @line_item, :status => :created,

:location => @line_item }

  else

    format.html { render :action => "new" }

    format.xml  { render :xml => @line_item.errors, :status =>

:unprocessable_entity }

  end

end

end

so I started by doing this

line_item.rb***********

def decrease_seat_capacity_on_create

  new_capacity =  new_capacity - @line_item.quantity

  event.capacity = new_capacity

  event.save

end

In this method @line_item does not exist because you have not created it. So you may need to pass to this method the line_item object or look it up.

Also, you should look at the after_save callback in ActiveRecord, that way you might be able to tell your LineItem model to automatically perform this action after a successful save (if you google for activerecord callbacks you will find a number of them, they are very useful).

3

See inserted comments below.

Quoting linda Keating <lists@ruby-forum.com>:

I'm learning with the Agile Web Development 4th edition book. I've built a method in my controller to test that it works, and now I want to move it to the model. ******************************** *** line_item controller *** ******************************** def create     @cart = current_cart      event = Event.find(params[:event_id])      @line_item = @cart.add_event(event)       new_capacity = event.capacity

    respond_to do |format|       if @line_item.save         new_capacity = new_capacity - @line_item.quantity         event.capacity = new_capacity         event.save

        format.html { redirect_to(@line_item.cart, :notice => 'Line item was successfully created.') }         format.xml { render :xml => @line_item, :status => :created, :location => @line_item }       else         format.html { render :action => "new" }         format.xml { render :xml => @line_item.errors, :status => :unprocessable_entity }       end     end   end

so I started by doing this

*********************************** *****line_item.rb**************** ********************************

def decrease_seat_capacity_on_create

      new_capacity = new_capacity - @line_item.quantity       event.capacity = new_capacity       event.save   end

new_capacity and event are local variables in the controller method. They are not accessible in the called model method. @line_item is an instance variable of the controller instance. It is also not accessible in the model method. However it references the same object as self in the model method. Presumably, event is another model. Note: it is possible that a LineItem has an event method because there is an association between Events and LineItems.

I don't see any good reason to push this functionality into a model method. The only property of the model that is clearly being used is the quantity attribute/method. Unless the LineItem model includes a reference to an Event (i.e. LineItem belongs to an Event or hasMany Events). It still looks like poor design unless there are several factors not mentioned here.

Jeffrey

and made a changed the line_item controller like this

respond_to do |format|       if @line_item.save         @line_item.decrease_seat_capacity_on_create

But I get the error

undefined method `quantity' for nil:NilClass Rails.root: c:/webtech/ServerSide/tickets_online

Application Trace | Framework Trace | Full Trace app/models/line_item.rb:12:in `decrease_seat_capacity_on_create' app/controllers/line_items_controller.rb:51:in `block in create' app/controllers/line_items_controller.rb:49:in `create'

So I'm a bit confused about 'quantity'. I've used it in another method in the line_item.rb and it worked (much to my surprise) but it doesn't work in this new method I'm writing.

It isn't quantity that isn't working, it is that @line_item is nil (i.e., @line_item of the LineItem instance has not been assigned a value). @line_item is an instance variable in the controller, not in the model.

I really am an absolute beginner so I'm really just guessing most of the time. Any suggestions or hints would be most appreciated.

HTH,   Jeffrey

4

Jeffrey L. Taylor wrote in post #997405:

See inserted comments below.

Quoting linda Keating <lists@ruby-forum.com>:

      new_capacity = event.capacity         format.xml { render :xml => @line_item, :status => :created,

new_capacity and event are local variables in the controller method. They are not accessible in the called model method. @line_item is an instance variable of the controller instance. It is also not accessible in the model method. However it references the same object as self in the model method. Presumably, event is another model. Note: it is possible that a LineItem has an event method because there is an association between Events and LineItems.

I don't see any good reason to push this functionality into a model method. The only property of the model that is clearly being used is the quantity attribute/method. Unless the LineItem model includes a reference to an Event (i.e. LineItem belongs to an Event or hasMany Events). It still looks like poor design unless there are several factors not mentioned here.

Hi Jeffrey, I was moving it into the model soley because I've read that fat models and skinny controllers are the best design practice. I was just trying to get the function to work first, and now that it's working I was hoping to tidy up the design. But maybe I should just leave it as is. I have though similar functionality in the update controller that looks like this ...

def update     @line_item = LineItem.find(params[:id])     event = @line_item.event     new_quantity = params[:line_item_quantity].to_i     change_in_capacity = @line_item.quantity - new_quantity

    respond_to do |format|       if @line_item.update_attributes(params[:line_item])         if (@line_item.quantity >0)          event.capacity = change_in_capacity          event.save

and I'll be writing something similar again reversing the action in the destroy method of line_item.

But maybe I should just leave it as is for the moment until I've learned a bit more about Ruby and Rails.

Many thanks for your help and feedback. Really appreciate your patience.

Linda