This code is ugly, how to do better?
todos = Todo.all
@todos_1 = Array.new
@todos_2 = Array.new
@todos_3 = Array.new
todos.each do |doc|
if doc.category == nil
@ntodos_1 << doc
end
if doc.category == "something"
@todos_2 << doc
end
if doc.frame == "whatever"
@todos_3 << doc
end
You can use Todo.group("category").order("category") to organize the result set and then loop over it, knowing that when category changes you are at the next grouping.
Alternatively, it might be useful to create scopes for the Todo model:
class Todo < ActiveRecord::Base
scope :something, where(:category => "something")
scope :whatever, where(:category => "whatever")
end
This would allow you to assign the results to instance variables instead of iterating over all results within your controller:
@something = Todo.something
@whatever = Todo.whatever
Complementing the existing answer, let's assume you are working with "normal" Ruby objects, not a ORM:
todos_by_category = todos.group_by(&:category)
To be used:
>> todos_by_category["some_category"]
#=> [todo1, todo2, ...]
At least the first two if's could be combined in one case:
todos.each do |doc|
case doc.category
when nil
@ntodos_1 << doc
when "something"
@todos_2 << doc
end
if doc.frame == "whatever"
@todos_3 << doc
end
You also know elsif? }
doc.frame == 'whatever' && doc.category == 'something'? Can you please provide a proper specification of what the code should be doing, preferably including test cases?