1
2
3
4

def email
  return '' unless logged_in?
  current_user.email
end

user.rb

1
2
3
4

def carrier?
  carrier_id != 0
end

original submission

1
2
3
4

@email     = logged_in? ? current_user.email     : ""
@mobile_no = logged_in? ? current_user.mobile_no : ""
@carrier   = logged_in? && current_user.carrier? ? current_user.carrier.name : ""

user.rb

1
2
3
4
5
6
7
8
9
10
11

class User < ActiveRecord::Base

  def carrier?
    ...
  end

  def carrier_name
    carrier.name if carrier?
  end

end  

controller

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

# 1. I don't think you're gaining anything by setting these variables to empty strings when the user isn't logged in. nil should do the job just fine.

  def show
    if logged_in?
        @email               = current_user.email  
        @mobile_no      = current_user.mobile_no 
        @carrier_name = current_user.carrier_name
     end
   end

# 2. I tend to only assign records to separate instance variables, rather than assigning individual attributes. Assigning attributes can quickly get confusing and difficult to manage. In this case I think it's fine, if that's what you're comfortable with, as long as you're not assigning to more than 3-4 instance variables in your controller.

  def show
    # My preference. No need for any of that code in the controller...
  end