views:

282

answers:

4

Hi all,

I have an app that models a House. The House has_many Rooms, Rooms has_many Lights and Small_appliances, etc. I also have a controller called Calculator that is how the app is accessed. Data is added to the house (and its rooms) using the Calculator controller. Then a report is generated, which is located at app/views/calculator/report.html.erb.

My question is where should all the calculations and logic for the report go? Currently I have it all in the view, with some things in calculator_helper. Normally this would go in the model, right? But Calculator doesn't have a model that was generated. What is the standard for this?

Here is the calculator controller.

class CalculatorController < ApplicationController
  def index
  end

  def save_house
    @house = House.new(params[:house])
    respond_to do |format|
      if @house.save
        format.html { render :action => 'add_rooms', :id => @house }
        format.xml { render :xml => @house, :status => :created, :location => @house }
      else
        format.html { render :action => 'index' }
        format.xml  { render :xml => @house.errors, :status => :unprocessable_entity }
      end
    end
  end

  def add_rooms
    @house = House.find(params[:id])
    @rooms = Room.find_by_house_id(@house.id)

  rescue ActiveRecord::RecordNotFound
    logger.error("Attempt to access invalid house #{params[:id]}")
    flash[:notice] = "You must create a house before adding rooms"
    redirect_to :action => 'index'
  end

  def add_room
    @room = Room.new(params[:room])
    @house = @room.house

    respond_to do |format|
      if @room.save
        flash[:notice] = "Room \"#{@room.name}\" was successfully added."
        format.html { render :action => 'add_rooms' }
        format.xml { render :xml => @room, :status => :created, :location => @room }
      else
        format.html { render :action => 'add_rooms' }
        format.xml  { render :xml => @room.errors, :status => :unprocessable_entity }
      end
    end
  rescue ActiveRecord::RecordNotFound
    logger.error("Attempt to access invalid house #{params[:id]}")
    flash[:notice] = "You must create a house before adding a room"
    redirect_to :action => 'index'
  end

  def report
    flash[:notice] = nil
    @house = House.find(params[:id])
    @rooms = Room.find_by_house_id(@house.id)
  rescue ActiveRecord::RecordNotFound
    logger.error("Attempt to access invalid house #{params[:id]}")
    flash[:notice] = "You must create a house before generating a report"
    redirect_to :action => 'index'
  end
end
A: 

This all depends on what kind of data you're creating. What does the calculator controller look like?

You can create your own classes in /lib and use them in your models, which can be a good way to separate out logic from the controller/helpers. Is there a reason why you couldn't put some of the logic in the models?

jonnii
calculator_controller saves data and moves the user to the next page that allows them to enter more information. The models are doing validation, but that's about it. The calculations that are being performed are mainly number calculations based on house and room parameters. I tried making a model for calculator, but then I can't access the variables from the view. Would I have to make then all global variables?
Ryan
+1  A: 

I would create a class in RAILS_ROOT/lib/ called, for example, Calculator and put the code in there.

Classes in /lib/ should be loaded an available anywhere in your app.

You can also create a plain ruby object in /app/models/. There's no reason they all have to inherit from ActiveRecord::Base

BJ Clark
I've made a calculator.rb file in app/models that doesn't not inherit from ActiveRecord. However, when I move my code to that file I can no longer access it from the view. I'm assuming it's because I did it all with local variables. What is the best way to fix this? I tried changing them to global variables, but it doesn't seem to help.
Ryan
Putting the methods into the models, instead of a class in lib/, better ties the logic to the underlying data. Also, it has the distinct advantage of not suffering from this problem relating to instance variables since you can call the methods on the instances of the models you are already handling in the controller and views.Farming code out into lib/ makes the most sense if it is used by multiple classes in your codebase, such as a Mixin Module, or is something you are refactoring out into a reusable sidecar project/plugin. If the logic is part of the codebase, keep it with your main code.
James Thompson
+2  A: 

There are a few ways to approach it, but the logic certainly does not belong in the view. You have the various models associated with one another in a clear hierarchy with the top of the hierarchy being the House model, if I am reading your description correctly. That being the case, I would add an appropriate method of set of methods to the House model that may be composed of calls to calculation methods in the Room models associated with a given House instance and on down the line of association. That ways the relevant calculation can be performed at each level and through composing one or more methods at the House model level you are able to have a clean, expressive and maintainable way to deal with calculations.

One thing to do, as well, would be to make sure that any calculations that can be performed by the DB are. For example, if there is a calculation that a Room model can do by simply querying it's own data then by all means push that computational burden to the DB using the ability of ActiveRecord to invoke such lower level calculation logic. Check out the API docs for the details.

I would look very carefully at the logic you want and see how it can be pushed into the model since that is probably where it belongs, close to the actual data of the calculations, and within the class structures that represent that data specifically; I would not create a model just to handle the calculation logic unless you really need to store the calculations persistently for some reason.

James Thompson
A: 

Ok, now I can see the code posted. I can see the calculator_controller actually has no calculations in it, are they in the views?. Try this approach:

  1. Write a test that sets up an object which will return the results you need to return to the user of the web page, given a house, rooms or whatever else it needs.
  2. Build a model (in models) to make that test pass.
  3. Modify your controller code above to use your new calculator model
  4. Modify the tests of your controller so they also pass. These tests, of course, do not need to test any business logic.

My prior respose:

If the business logic is fairly simple and only used behind this web app, then you can put it in your app/models folder.

class MyCoolClass
  def initialize(clues)
    @other_things = OtherThing.all
  end
  def do_cool_thing; end
  def calculate_coolness
    @other_things.length 
  end
end

Then in your controller, create an instance of your model

def index
  @mcc = MyCoolClass "A clue as to what I want"
  render
end

Then in your templates you can access it

<%=h @mcc.calculate_coolness %>

Note that @other_things is an *instance__variable* of MyCoolClass and generally not accessible to the templates without accessor methods being defined

wentbackward
I like building models like this when the logic begins to clutter up the AR model.
Jonathan Julian