From: Peter Ertl <pertl@...>
Date: 2006-06-13T19:22:37+09:00
Subject: Re: ugly ruby code...

Maybe like that:

---

class Room < ActiveRecord:Base
  has_many :racks
end

class Rack < ActiveRecord:Base
  # uses self.room_id for reference
  belongs_to :room
end

---

used_racks = Room.find(params[:id])racks
free_racks = Racks.find_all - used_racks



> >   @racks = Rack.find( :all )
> >   @racks_room = Array.new
> >   @free_racks = Array.new
> > 
> >   for rack in @racks do
> >     if rack.room_id == @room.id
> >       @room_s_racks << rack
> >     else
> >       @free_racks << rack
> >     end
> >   end




-------- Original-Nachricht --------
Datum: Tue, 13 Jun 2006 19:08:13 +0900
Von: Robin Stocker <robin@nibor.org>
An: ruby-talk@ruby-lang.org
Betreff: Re: ugly ruby code...

> arnaud stageman wrote:
> > def show
> >   @room = Room.find(params[:id])
> >   @racks = Rack.find( :all )
> >   @racks_room = Array.new
> >   @free_racks = Array.new
> > 
> >   for rack in @racks do
> >     if rack.room_id == @room.id
> >       @room_s_racks << rack
> >     else
> >       @free_racks << rack
> >     end
> >   end
> > end
> 
> Hi again,
> 
> Just some minor suggestions:
> 
> You seem to use only instance variables. If you are not going to need 
> them later in views, I would prefer using local variables because they 
> go out of scope when the method finishes and don't clutter the object 
> with unnecessary variables. So, just replace @racks with racks.
> 
> Replace the long Array.new with the shorter form []. Of course this is 
> just my opinion, but I find it easier to type and easier to read as it 
> gives you some kind of visual clue that there's an array involved.
> 
> ...my 0.02 Swiss francs ;)