Optimizing Rails Code

Hello,

I often find myself creating and retrieving ActiveRecord objects that
barely get used, unaware of perhaps a better way of database
interaction within Rails. Are there more efficient ways to do some of
the following things:

Ex 1:
Supplier.find(supplierid).update_attributes(:num_products =>
total, :ship_fee => SHIPPING_FEE)

Ex 2:
      item = Product.find(
        :first,
        :conditions => ["supplierid = #{supplierid} AND sku =
'#{attributes[:sku]}'"]
      )
      attributes[:last_change] = Date.today if product_changed(item,
attributes)
      attributes[:last_update] = Date.today
      item.update_attributes(attributes)

Ex 3:
      image = SupplierImage.new(
        :productid => productid,
        :mid => IMAGE_MACHINE,
        :filename => filename,
        :alt => prodname,
        :supplier_url => url
      )
      image.save

Thanks!

In the third instance, you could just use "create" instead of
"new/save". Doesn't save you many lines (1) and doesn't really make
it any more efficient (all that does is call new/save), but hey at
least it looks better. :slight_smile:

--Jeremy

Ex 1:
Supplier.find(supplierid).update_attributes(:num_products =>
total, :ship_fee => SHIPPING_FEE)

Supplier.update(supplierid, :num_products => total, :ship_fee => SHIPPING_FEE)

Ex 2:
      item = Product.find(
        :first,
        :conditions => ["supplierid = #{supplierid} AND sku =
'#{attributes[:sku]}'"]
      )
      attributes[:last_change] = Date.today if product_changed(item,
attributes)
      attributes[:last_update] = Date.today
      item.update_attributes(attributes)

Not clear about this one.

-Pratik

Matt White wrote:

Ex 2:
      item = Product.find(
        :first,
        :conditions => ["supplierid = #{supplierid} AND sku =
'#{attributes[:sku]}'"]
      )

You are leaving yourself open to SQL injection here. You should always use :conditions => ['supplierid = ? AND sku = ?', supplierid, attributes[:sku]]

Also, you don't need to do a find if all you want to do is to update an attribute, checkout update_all for that.

> Ex 1:
> Supplier.find(supplierid).update_attributes(:num_products =>
> total, :ship_fee => SHIPPING_FEE)
>

Supplier.update(supplierid, :num_products => total, :ship_fee => SHIPPING_FEE)

This saves you a line of code. If all you want to do is update some
fields quickly, use update_all, though it's closer to raw sql.

Supplier.update_all ['num_products = ?, ship_fee = ?', total,
SHIPPING], ['id=?', supplierid]

Though no validations or callbacks are called on your model.

> Ex 2:
> item = Product.find(
> :first,
> :conditions => ["supplierid = #{supplierid} AND sku =
> '#{attributes[:sku]}'"]
> )
> attributes[:last_change] = Date.today if product_changed(item,
> attributes)
> attributes[:last_update] = Date.today
> item.update_attributes(attributes)

Same thing, all you're doing is updating a few timestamps on a record.