Beginners question | moving a method from controller to model

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.

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).

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

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