if @user && @user.role && @user.role.eql?('user') && @user.user_group
@group_name = @user.user_group.name
end
Refactorings
No refactoring yet !
Adam
August 13, 2010, August 13, 2010 17:39, permalink
No rating.
Login to rate!
Note: If Object#try is not already defined, you can grab it out of ActiveSupport.
@group_name = @user.user_group.try(:name) if @user.try(:role) == 'user'
Adam
August 13, 2010, August 13, 2010 18:14, permalink
1 rating.
Login to rate!
On second thought, you might be trying to do too much in one place. I'm thinking, in absence of the rest of the code, that you probably do not want the program flow to even make it this far in the code if @user is undefined.
class User
delegate :name, :to => :user_group, :prefix => :user_group, :allow_nil => true
def user?
role == 'user'
end
end
@group_name = @user.group_user_name if @user.user?
chipcastle.myopenid.com
August 13, 2010, August 13, 2010 19:15, permalink
No rating.
Login to rate!
Great ideas! If I'm following you, shouldn't line #9 be this instead?
@group_name = @user.user_group_name if @user.user?
It seems to me that there should be a way to leave out the "@user && @user.role". Possibly also the "&& @user.user_group" part. Any suggestions?