Created
April 24, 2013 18:06
-
-
Save pitosalas/5454177 to your computer and use it in GitHub Desktop.
This is a simple bit of code that I think is ugly. I am looking for a suggestion on making it more idiomatic. Thanks!
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
def populate_todolist | |
progs = @user.participating_programs | |
@to_do_list = progs.map { |prog| build_todolist_row(prog) } | |
@to_do_list.compact! | |
end | |
# return nil if analysis yields no todolist row. compact! above removes the nil. | |
def build_todolist_row prog | |
rounds = prog.open_rounds | |
raise "more than one round open in ToDoList" if rounds.length > 1 | |
if rounds.length == 1 | |
result = ToDoListRow.with(program_name: prog.name) | |
end | |
return nil | |
end |
@pitolas, i'm suggesting that it should be defined on programs, too. Or some method that encapsulates the notion of whether or not the program has more than one open round. Maybe the name isn't right; I don't know your domain.
I would change the API of these methods a little. Resulting in the following
def populate_todolist(programs)
@to_do_list = programs.map do |program|
build_todolist_row(program.name, program.open_rounds)
end
end
def build_todolist_row(program_name, rounds)
raise "more than one round open in ToDoList" if rounds.length > 1
ToDoListRow.with(program_name: program_name)
end
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Thanks! There was a bug due to a last minute untested tweak before I posted it so indeed the return null at the end is wrong.
Re: @arwegner: @to_do_list = @user.participating_programs.select(&:open?).map(&:build_to_do_list)
Looks promising, but the "open" method is on rounds which belong_to programs.
Re: @hurshawrawai: See my first sentence here. The reason I had two methods is that the logic for creating a ToDoListRow is going to get much more complex, and, in fact, sometime after deeper probing it might turn out that for a particular prog there is no to ToDoListRow needed.
Thanks!
Pito