Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Should I move my custom methods to model from controller?

Let's say I have a Product model and ProductsController. Controller has all the standard CRUD method and Product does all kinds of validations etc.

Here is an issue. I have several custom very complex actions which also need to respond in multiple formats( json, html, xml, csv, pdf etc). Business logic reasons for this are beyond the scope of the question. Let's just way this is the way it has to be done. Also I use InheritedResources gem but I don't think it matters for the question.

For example ( this is a mock app, which is GREATLY simplified - I removed all kinds of if else statements and loops and localization, etc ):

class ProductController < InheritedResources::Base
  ....
    def check_stock_using_legacy_identifier_and_create_a_unique_po_number_and_place_an_order
      @order = Order.new
      @product = Product.find(params[:legacy_alphanumeric_product_number])
      if @product.stock > 5
        @po = LegacyOrder.create_po
        if @po
          if @order.save
            format.html{ render :check_stock_using_legacy_identifier_and_create_a_unique_po_number_and_place_an_order, flash: {success: "Wow! Input was good!"}}
            format.json{ render status: 400, json: {status: :success, message: "Order created"}}
          else
            format.html{ render :check_stock_using_legacy_identifier_and_create_a_unique_po_number_and_place_an_order, flash: {error: "Can't create order, some validations failed"}}
            format.json{ render status: 400, json: {status: :error, message: "Problem with order", errors: @order.errors}}
          end
        else
          format.html{ render :check_stock_using_legacy_identifier_and_create_a_unique_po_number_and_place_an_order, flash: {error: "Can't create order, PO number wasn't generated"}}
          format.json{ render status: 400, json: {status: :error, message: "Problem with po", errors: @po.errors}}
        end  
      else
        respond_to do |format|
          format.html{ render :check_stock_using_legacy_identifier_and_create_a_unique_po_number_and_place_an_order, flash: {error: "Can't create order, stock is low"}}
          format.json{ render status: 400, json: {status: :error, message: "Problem with product", errors: @product.errors}}
        end
      end  
    end   
  ....
end 

This is just to give an idea of complexity of some actions.

Now the question: should all of that goodness be moved into model? I'm dealing with business logic, which should be in the controller, but with trying keep with the rule of thumb Fat Models & Thin Controllers, it seems to me that it should be moved away, if so then what there is left to move?

Bonus Q: I'm coming accross use cases where I might need to use some of that functionality in the code, not through a REST interface. I.E. I need to use check_stock_using_legacy_identifier_and_create_a_unique_po_number_and_place_an_order, while running a rake task. Like generating some orders based on low stock, or from an email event, etc. While I can use options described here: How do I call controller/view methods from the console in Rails? , having this action as part of a model would make it easier wouldn't it?

So what is a Rails Best Practices course of action in a case like this?

like image 291
konung Avatar asked Jun 03 '13 23:06

konung


2 Answers

Consider moving your logic into a service object. I think shoving controller logic into the model is simply moving the problem to a different location. Yes, you do isolate the logic to a single area, but there are instances where you end up moving logic to models because of convention rather than the fact that it really belongs there.

A service object can help you reduce duplication and isolate your business logic without getting the model too involved in things it does not need to know about (your repeated json response, for example).

class OrderService
  def initialize(legacy_alphanumeric_product_number)
    # do stuff
  end
  # do more stuff
end

From the controller, you can just call

def check_whatever
  @order = OrderService.new(params[:some_product_code])
  @order.check_something
  # do more stuff
end

Take a look at 7 Patterns to Refactor Fat ActiveRecord Models. I found it very helpful. There is also a RailsCasts episode on service objects (requires pro subscription).

like image 126
Mohamad Avatar answered Sep 18 '22 22:09

Mohamad


Take a look at the Draper gem (https://github.com/drapergem/draper). It provides nice decorator style wrappers that are the perfect place for putting logic on what to show or respond to. I agree with @mohamad that it doesn't belong in the model or controller. A service object is definitely a good way to go, or using Draper to create presentation specific logic methods.

like image 32
Alex Peachey Avatar answered Sep 19 '22 22:09

Alex Peachey