11

Some recommendation [1] suggest you to use

<%= current_user.welcome_message %>

instead of

<% if current_user.admin? %>
  <%= current_user.admin_welcome_message %>
<% else %>
  <%= current_user.user_welcome_message %>
<% end %>

But the problem is you must have the decision logic somewhere in your code.

My understanding is putting the decision in template is better than controller as it make your controller more clean. Is it correct?

Are there better way to handle this?

http://robots.thoughtbot.com/post/27572137956/tell-dont-ask

tereško
  • 58,060
  • 25
  • 98
  • 150
Howard
  • 19,215
  • 35
  • 112
  • 184

10 Answers10

11

You are not the first to wonder this. If views and controllers should have little to no logic, and the model should be presentation agnostic, where does presentation logic belong?

Turns out we can use an old technique called the decorator pattern. The idea is to wrap your model object with another class that contains your presentation logic. This wrapper class is called the decorator. The decorator abstracts away logic from your view, while keeping your models isolated from their presentation.

Draper is an excellent gem that helps define decorators.

The sample code you gave could be abstracted like so:

Pass a decorator to the view with @user = UserDecorator.new current_user in your controller.

Your decorator could look as below.

class UserDecorator
  decorates :user

  def welcome_message
    if user.admin?
      "Welcome back, boss"
    else
      "Welcome, #{user.first_name}"
    end
  end
end

And your view would simply contain @user.welcome_message

Notice that the model itself doesn't contain the logic to create the messages. Instead, the decorator wraps the model and translates model data into a presentable form.

Hope this helps!

cjhveal
  • 5,668
  • 2
  • 28
  • 38
  • I find this answer unsatisfying. Most of the time you will have many branches on admins, regular users, or guests (not signed ins), so you don't want one decorator that branches on #welcome_message, but rather many objects, some decorators and others perhaps NullObjects that all respond to #welcome_message. This way you only ask what kind of user it is once. – hgmnz Aug 04 '12 at 19:41
  • Had the asker mentioned that they had two classes for users, I'd have made two decorators. Instead, I opted to preserve the interface that was provided. The cost of the delegation across the decorator is fairly trivial compared to the clarity gained by separating data and presentation logic. – cjhveal Aug 05 '12 at 07:11
  • Sure, but you're starting off going down the wrong path. All I'm saying is I wouldn't write it this way even if so far I only had the one message to support, but maybe that's just me. – hgmnz Aug 05 '12 at 17:22
  • 2
    Fair enough. I was simply aiming to introduce the concept of decorators in a way with the least cognitive friction for the asker. – cjhveal Aug 06 '12 at 22:16
5

I would use a helper for this. Suppose you have to translate the welcome-message, based on some locale.

In the app/helper/user_helper.rb write

module UserHelper

  def welcome_message(user)
    if user.admin?
      I18n.t("admin_welcome_message", :name => user.name)
    else
      I18n.t("user_welcome_message", :name => user.name)
    end
  end 

end

and in your view you can then just write

<%= welcome_message(user) %>

Note that the decorator/presenter offers a really clean object-oriented approach, but imho using a helper is much simpler and sufficient.

nathanvda
  • 49,707
  • 13
  • 117
  • 139
4

No, you don't want any conditionals at all in the user class nor the controller. The point of that example on the blog post is to make reference to polymorphism, just good old fashioned OO design.

# in application_controller for example
def current_user
  if signed_in?
    User.find(session[:user_id])
  else
    Guest.new
  end  
end

#app/models/user.rb
class User
   def welcome_message
     "hello #{name}"
   end
end

#app/models/guest.rb
class Guest
  def welcome_message
    "welcome newcomer"
  end
end

...you get the idea.

Only, instead of littering your model with presentation-only methods, create a decorator that acts as a presenter:

require 'delegate'
class UserPresenter < SimpleDelegator
  def welcome_message
    "hello #{name}"
  end
end

And now current_user looks like so:

# application_controller
def current_user
  if signed_in?
    UserPresenter.new(User.find(session[:user_id]))
  else
    Guest.new
  end
end
hgmnz
  • 13,208
  • 4
  • 37
  • 41
  • I think view code in the model is as bad as having it in the controller. Would recommend having those methods in a decorator instead, as @adriandz mentioned in his answer. – Tanzeeb Khalili Jul 30 '12 at 05:34
  • Tanzeeb, if you had actually read my entire answer, you'd know that I have the same opinion regarding putting presentation logic in the model. – hgmnz Jul 30 '12 at 15:44
  • 2
    You should have that as your actual recommendation, not a byline at the end. The OP is asking for best practices, this is your opportunity to teach him. – Tanzeeb Khalili Jul 30 '12 at 22:02
3

Decorate the user model and add the welcome_message to it directly. Yes, this may involve some kind of conditional statement at some point.

http://robots.thoughtbot.com/post/14825364877/evaluating-alternative-decorator-implementations-in

adriandz
  • 999
  • 1
  • 10
  • 21
1

In my opinion, if the text is the only thing that changes, it doesn't belong in a view. If you needed to restructure the page, that's presentation logic. This, this is just data being different.

Amadan
  • 191,408
  • 23
  • 240
  • 301
  • What if the text needs to be localised? – nathanvda Aug 02 '12 at 13:53
  • @nathanvda: Then you translate it. `@welcome_notice = I18n.t(@user.admin? ? :welcome_notice_for_admins : :welcome_notice_for_users, :name => @user.name)`. Text translations, except for those that are constant (i.e. arguably part of the design) should not be in views. – Amadan Aug 02 '12 at 16:14
  • @nathanvda: Oh... Just read your answer, in which you say essentially the same. Then I don't understand your question. We both agree this shouldn't be in a view. – Amadan Aug 02 '12 at 16:18
  • So you would place the `I18n.t` calls in the model? In the controller? – nathanvda Aug 02 '12 at 21:24
  • 1
    @nathanvda: Model, controller, decorator, anywhere but view. Decorators might be a bit tough for beginners, so I wouldn't be too bent out of shape if it was in the controller. It's a bit too presentationy for the model, but even that is better than the view. My preference would probably be something like cjhveal's answer, with `t()` calls inserted if i18n is needed (even though my very quick example code three comments up is written from the controller point of view). – Amadan Aug 03 '12 at 01:05
  • Ok. I would say anywhere but the model :) The model has no access to current language and should not know. I do not like something like that in the controller, since it is just a detail. I would put it in a helper/decorator or partial or view. I presume you prefer your views to be totally logicless? – nathanvda Aug 03 '12 at 08:14
  • @nathanvda: Yeah, the choice between `:welcome_notice_for_users` and `:welcome_notice_for_admins` can go into model, actual translation not. But I'd prefer if it didn't. On logicless views: I prefer my views to only handle presentation; text is data, unless it's part of the design. E.g. when displaying a table, having an `if .empty?` and `.each` are OK in the view. The text displayed on headers is likewise design; it can go in the view. A welcome message though is not a part of the design in my mind. It's fuzzy, but I know what I mean :p – Amadan Aug 03 '12 at 09:19
1

I think you should watch the railscasts episode on Presenters for the answer.

user864652
  • 34
  • 6
1

Logic in the view is hard to maintain, we should put the business logic in the model and all view logic in helpers.

If you want your code to be in Object Oriented fashion, make use of Decorators (object oriented way of helpers)

Best Example : https://github.com/jcasimir/draper

pdpMathi
  • 777
  • 3
  • 7
0

Put the code defining current_user.welcome_message in _app/helpers/application_helper.rb_, it will then be accessible by any view rendered with the application layout.

Another option is to define a custom helper module, one which is not necessarily associated with a given view or controller (See the video I linked below), and include it in the modules of the view/controllers you wish to have that functionality in.

This is not something that is black and white. But, from what you have described it sounds like this is code that is obtrusive to stick in your application_controller.rb and it is not code with functionality which justifies it's own controller, the most effective and efficient option may be to create a custom helper module and include it in the helpers you wish to have that functionality. That said, this is ultimately a judgement call which the designer of the application (i.e. you) needs to decide upon.

Here is a good article outlining helper modules from May, 2011

Here is is a RailsCast outlining custom helper modules (i.e. custom as in modules not necessarily associated with a given controller or view). Short, sweet, and to the point.

rudolph9
  • 8,021
  • 9
  • 50
  • 80
  • Using a helper method is not at all the intent of the article the OP linked to. – adriandz Jul 29 '12 at 12:15
  • @adriandz Right, but where do you define the logic fo `current_user.welcome_message`? The question isn't asking if one approach is better than the other (obviously `current_user.welcome_message` is better than the other in your view) but is asking where is that that kind of logic supposed to go? – rudolph9 Jul 29 '12 at 12:20
0

You can define helper method for that stuff. I don't think it is a good Idea to make a welcome sentences in a model, but in the controller too. But you should try to make you views clean from code, and if you can use helpers for that then you should to.

freeze
  • 546
  • 6
  • 16
0

A good practice would be to have real View instances. Rails parody of MVP (there is difference, look it up) unfortunately seems to pretend that views are templates. That is wrong.

Views are supposed to contain the presentation logic in MVC and MVC-inspired patterns. They are also supposed to manipulate multiple templates and make decision on which templates to employ to represent the state and information from the model layer (yes, model is a layer not an ORM instance).

So, to answer the question: presentation logic has no place in controllers.

tereško
  • 58,060
  • 25
  • 98
  • 150