Rails 4: Is it a bad practice to do database operations in model?

Question

Say I have product model(app/models/product.rb) and the following method def in the model.

def update_stock product
  product.stock -= 1
  product.save(:validation => false)
end

Is this acceptable?

Thanks in advance.

Answer 1

This looks suspicious to me - you say that this is a method in your Product model, but then the method updates an entirely different product instance (the one you pass in, not the one you call the method on).

If this was something like:

class Product < ActiveRecord::Base
  def update_stock
    self.stock -= 1
    save
  end
end

Then it would be much more appropriate. (update_stock doesn't seem like the best name for the method, either, and skipping the validations likely isn't a good idea.)

Answer 2

This is a good practice to keep the database operations and business logic of an application in model. Model directly interacts with database with ORM (Object Relational mapping). Also in rails convention controller should be thin and model should be fat .This answer explains it in details

What is Ruby on Rails ORM in layman's terms? Please explain

Answer 3

As you want to update something better use update_attributes!

It may be write in this way:

class Product < ActiveRecord::Base

  def update_stock product
   self.update_attributes!(stock:  stock - 1)
 end
end

Answer 4

It's up to you!

The simplest way I use to deduce how to do things is to look at it from a modular perspective. If you have "heavy" code in a controller, will you benefit from using that code in another area of the app?

---

Rails is a series of classes & modules

Code is accessible to different parts of the app, depending on which modules are called. If you feel like you'll benefit from re-using the method in different areas of the app, you should include either an instance or class method in the model, as this will open this method to those objects

I would use the code provided by RSB & sevenseacat, probably using the decrement! method in sevenseacats answer:

self.decrement!(:stock)

---

In regards to your question, I would personally do that, but would make it an instance method. sevenseacat basically explained what I'd do