align-toparrow-leftarrow-rightbackbellblockcalendarcamerachatcheckchevron-downchevron-leftchevron-rightchevron-small-downchevron-small-leftchevron-small-rightchevron-small-upchevron-upcircle-with-crosscrosseditfacebookglobegoogleimagesinstagramlocation-pinmagnifying-glassmailmoremuplabelShape 3 + Rectangle 1outlookpersonplusImported LayersImported LayersImported Layersshieldstartwitteryahoo

Re: [ruby-81] copying and comparing model objects

From: William S.
Sent on: Tuesday, March 4, 2008 2:00 PM

I would refactor the code and have it reside in the model, not in the controller. You can create an after_find method or explicately tell the model to save the state. Then you can create a method on the model to check if it is changed using a method like changed? to compare the hashes. This will be a better separation of responsibilities. The controller shouldn't know or want to know that the model has attributes.

On Mar 4, 2008, at 1:37 PM, Loqi wrote:

I would suggest not creating a separate object, but after you find the object, dup the @attributes and save them as something called for example @original_attributes. Before you save the object, iterate and diff the two hashes. This will avoid having to deal with two completely separate model objects and still preserve the original state. has_one and has_many relationships withstanding.

I think I got the first part. (I forgot .dup in my first attempt).

def edit
  @product = Product.find(params[:id])
  @product_attributes_before = @product.attributes.dup # <<<<<<<<<<<< right?
end

But what do you mean about iterating the hashes? Doesn't Hash#diff do that for me?

def update
  if @product.attributes.diff(@product_attributes_before) == {} # <<<<<<<<<<< right?
    flash[:notice] = 'You haven't changed anything.'
    redirect_to :action => 'show', :id => @product
  else
    @product = Product.find(params[:id])
    if @product.update_attributes(params[:product])
      flash[:notice] = 'Product was successfully updated.'
      redirect_to :action => 'show', :id => @product
    else
      render :action => 'edit'
    end
  end
end

Should I be doing something more on my .diff line above?

I should have asked what you were planning on doing before offering a solution. In the update method you just want to ask if the model has changed. You could have created a method that said:

class Product < ActiveRecord::Base
  ...

   def after_find
       @saved_attributes = @attributes.dup
   end

   def changed?
@attributes.diff(@saved_attributes).empty?
   end
end

If you don't want this to happen automatically, you can replace the after_find with your own method.

You still have to do the product find. When you come into the update method, the controller instance will be new and you won't have access to @product_attributes_before. Unless you save this object in session, it will be lost, and that's a good thing! I would rewrite the code:

def update
    @product = Product.find(params[:id])
           @product.update_attributes(params[:product])
  if !@product.changed?
    flash[:notice] = 'You haven't changed anything.'
    redirect_to :action => 'show', :id => @product
  else
    if @product.save
      flash[:notice] = 'Product was successfully updated.'
      redirect_to :action => 'show', :id => @product
    else
      render :action => 'edit'
    end
  end
end


Was your point to try to avoid the find? This is something you probably don't want to do. Problems with consistency and session abuse any many other issues. 

This solution could be easily captured as a mixin.

Eventually I'll learn how to properly use mixins.

Cheers,
- Will

Our Sponsors

People in this
Meetup are also in:

Sign up

Meetup members, Log in

By clicking "Sign up" or "Sign up using Facebook", you confirm that you accept our Terms of Service & Privacy Policy