Topic: Refactoring my code

Let say i have 3 models ,name ModelA, ModelB, and ModelC
In ModelAController i have this

def index
    @name_a = ModelA.find_by_name(:last)
    @name_b = ModelB.find_by_name(:last) 
    @name_c = ModelC.find_by_name(:last)
  end

and in view i display the result like this
 
  Person A = <%= name_a %>
  Person B = <%= name_b %>
  Person C = <%= name_c %>

My question is, Is there a way to refactor my code above ?

Re: Refactoring my code

1. you can't use :last in a find_by finder
2. you can'T print out a model in the view, only it's attributes.

your code example should be working for us to refactor it wink

Re: Refactoring my code

Sorry for that, ok here is the updated code

  def index
    @name_a = ModelA.find(:first, :order =>"id desc").name
    @name_b = ModelB.find(:first, :order =>"id desc").name
    @name_c = ModelC.find(:first, :order =>"id desc").name
  end

and in modela/view/index
Person A = <%= @name_a %>
Person B = <%= @name_b %>
Person C = <%= @name_c %>

How ? Thank in advanced

Last edited by Gompuok (2008-07-07 03:55:20)

Re: Refactoring my code

IMO this is the best way to put it. There would probably by a DRYer way (e.g. using with_options - although I'm not sure if that works across different models), but nothing as readable. You should never give up clarity just for a little more DRYness.

Visit http://www.railway.at for quality Rails contract work. We also blog about Rails, AJAX and other things.

Re: Refactoring my code

clemens wrote:

IMO this is the best way to put it. There would probably by a DRYer way (e.g. using with_options - although I'm not sure if that works across different models), but nothing as readable. You should never give up clarity just for a little more DRYness.

I completely agree with clemens about clarity over DRYNess. 

However, just for the sake of answering the question, you could do this:

[ModelA, ModelB, ModelC].each do |klass|
  eval("@#{klass.to_s.underscore}_name = #{klass}.find(:first, :order => 'id desc').name")
end

Which would give you @model_a_name, @model_b_name, @model_c_name. 

I would just do the three seperate lines myself.  DRY shouldn't become an obsession, it's just a principle, not a law.

Last edited by Max Williams (2008-07-08 09:22:14)

###########################################
#If i've helped you then please recommend me at Working With Rails:
#http://www.workingwithrails.com/person/ … i-williams

Re: Refactoring my code

I agree with the others, but out of boredom:

{ :name_a => ModelA, :name_b => ModelB, :name_c => ModelC }.each do | ivar_name, klass |
  instance_variable_set( "@#{ ivar_name }", klass.find_by_name( :first, :order => "id desc" ).name )
end

Another way

find_first_name = lambda { | klass | klass.find_by_name( :first, :order => "id desc" ).name }
@name_a = find_first_name[ ModelA ]
@name_b = find_first_name[ ModelB ]
@name_c = find_first_name[ ModelC ]

And another, I guess

find_args = [ :first, { :order => "id desc" } ]
@name_a = ModelA.find( *find_args ).name
@name_b = ModelB.find( *find_args ).name
@name_c = ModelC.find( *find_args ).name

Now I really need to go and do something useful.

Last edited by Ninju (2009-01-28 13:12:06)

If I've helped you out with something, feel free to recommend me on working with rails.