51224bdd17878b3b19e8987e9bb336a2

Hi,
- In users table city column can have either city name or null.
- Then Many times in my code I need to write the following code.
- Instead of doing that can I set default very for city column as "Not specified".
Is it a good practice to do so?
- One more thing while showing the city I am calling capitalize method instead of that while saving user object shall I call before_save method and convert city to capitalize format.

Thanks
DG

user_profile.rhtml

1
2
3
4
5
6
7
              <%if @user.city.blank?%>
                Not specified
              <% else %>
                <%= @user.city.capitalize %>
              <% end %>
              

Refactorings

No refactoring yet !

Avatar

Ryan Bates

August 19, 2008, August 19, 2008 16:28, permalink

1 rating. Login to rate!

You could either do this at the model layer or a helper method. Or a combination of both which seems appropriate to me here:

1
2
3
4
5
6
7
8
9
10
11
12
# in user model
def display_city
  city.capitalize if city
end

# in helper
def blank_not_specified(value) # you can probably come up with a better name for this.
  value.blank? ? "Not Specified" : value
end

# in view
<%= blank_not_specified @user.display_city %>
F288a8afe5302a16a366d5e9d34f2fec

Joe Grossberg

August 19, 2008, August 19, 2008 20:40, permalink

1 rating. Login to rate!

No, it is not a good practice.

Instead you're better off doing this:

1
2
3
4
5
6
7
8
9
# in User model

def pretty_city
  city.blank? ? 'Not specified' : city.capitalize
end

# in your views

<%= @user.pretty_city %>
4418c10276dac6242f80773cf9445d39

Jake

August 19, 2008, August 19, 2008 21:26, permalink

1 rating. Login to rate!

What if you wanted to change the text "Not specified" to something else down the line? This is model logic and not view logic. Also, you should capitalize the city when you import it, or use CSS. There is no sense in continuously capitalizing something that should always be capitalized.

Of course, you could just set the column to have the default value of 'Not Specified'.

1
2
3
4
5
6
7
8
9
10
11
12
13
class User < ActiveRecord::Base

   def display_city
      return self.city.blank? ? 'Not Specified' : self.city
   end

end


<%= @user.display_city %>

<%# Or Without Editing Your Model %>
<%= @user.city.blank? ? 'Not Specified' : @user.city %>
A8d3f35baafdaea851914b17dae9e1fc

Adam

August 20, 2008, August 20, 2008 04:22, permalink

2 ratings. Login to rate!

I disagree with the others that it belongs in the model. This is most certainly display logic and would traditionally belong in a helper method. Except helper methods are rather ugly for this kind of thing. Thus, I give you a (basic) presenter:

Helper

1
2
3
4
5
module ApplicationHelper
  def present(object)
    object.class.const_get("Presenter").new(object)
  end
end

Presenter

1
2
3
4
5
6
7
8
9
class User::Presenter < Struct.new(:user)
  def city
    user.city.blank? ? "Not specified" : user.city.capitalize
  end
  
  def method_missing(method, *args, &block)
    user.send(method, *args, &block)
  end
end

View

1
<%= present(@user).city %>
51224bdd17878b3b19e8987e9bb336a2

DG

August 20, 2008, August 20, 2008 06:19, permalink

No rating. Login to rate!

Thanks all of you guys..

8991cd8e29cf99d2aebb2301c762e5bd

ivan.vanderbyl.myopenid.com

September 16, 2008, September 16, 2008 06:54, permalink

No rating. Login to rate!

View

1
<%= !@user.city.blank? && @user.city || "Not specified" %>

Your refactoring





Format Copy from initial code

or Cancel