2

So I'm working on a personal project on Rails to learn more about this framework, and I wanted to add a feature in which the user choose a particular record from the view, and after some calculations are made (which involves an array of 80 integers) I show the result... And because I find it looks ugly in the way I did I just wondered what was the convention to do this kind of things...

  1. In detail what I do is show you a list of records to the user, the user selects a record and a number, and then click the "Calculate exp" button...

one

  1. Then in the controller I check if the parameters exist, if they exist then I define the array and do the calculations based on the input number, on the record attributes and the array (the array never changes and could be defined as a constant in fact) that I define in the controller, and at the end I assing the result to an instance variable.

enter image description here

  1. Finally the result is shown on the same page, only if the instance variable that hold the result and is defined on the controller (after checking that the parameters exists) exists.

So my question is if I'm doing it the right way (is my first time trying to do something "serious" with rails and I don't know if I'm doing it right, especially by declaring an array of 80 elements in the controller, I don't like how it looks and I don't feel it's right.).

1

This is what a conventional rails controller would look like:

class HeroesController < ApplicationController
  before_filter :authenticate_user!
  before_filter :user_only

  def index
    @heroes = current_user.heroes.find_by_level(params[:level])
  end
end

Controllers should map to resources, and your controller is returning a list of heroes. Hence, HeroesController#index. All the finding stuff goes in the model.

  • The code in question isn't really finding a Hero, it is calculating some value based on the currently selected hero. It's hard to tell from the screenshot. – Greg Burghardt Mar 2 '15 at 18:58
0

I would say this is fine. You are using view templates, interacting with a controller, that's all good.

You might want to consider having a model and using that for the static data. Models will normally be database backed "Active Record" objects but do not have to be, they can also be used to hold the kind of data you have.

0

This calculation is Business Logic and should go in the Domain Layer of your application, i.e. your models.

The controller is the wrong place for this logic because it should be pretty dumb. It knows what to do (calculate a value) but not why it happens or the nitty gritty details of how it happens (the algorithm for the calculation).

A view model should just massage the domain model into something more palatable to the view, but it too should be devoid of business logic. The algorithm of the calculation should go in the model.

The screenshot of code you posted is too small to read, but it appears you have a Hero model. I would move the array of values into the Hero class and expose a public method that does the calculation and returns the result.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.