Optimizing Rails Code

rubyonrails-talk
1

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!

2

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

3

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

4

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.

5

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