# Repeating use of “v.” annoys me. Do you see a nicer way?

This code smells to me, but I don't see the way to clean it up. I see v. mentioned over and over. Any hints would be appreciated!

  #
# Create a new Value if it doesn't already exist, and initialize the attributes per
# the parameters of the call.
#
def self.find_or_create_value(prog_id, part_id, round_id, quest_id, new_value=nil)
prog = Program.find(prog_id)
attrs = {participant_id: part_id, round_id: round_id, question_id: quest_id}
v = prog.values.where(attrs).first
if v.nil?
v = prog.values.build
v.assign_attributes(attrs, without_protection: true)
end
v.value = new_value
v.save
v
end
end

-

You can use rails find_or_initialize_by method. Here is example:

  def self.find_or_create_value(prog_id, part_id, round_id, quest_id, new_value=nil)
prog = Program.find(prog_id)
prog.values.find_or_initialize_by_participant_id_and_round_id_and_question_id(part_id, round_id, quest_id) do |v|
v.value = new_value
v.save
end
end

-

Well I will repost my answer from stackoverflow for further discussion:

1. A method with over 4 parameters is a smell itself... that makes it unnecessarily complex. Maybe find objects that encapsulate some of the parameters (or: do you really need all of them?). Also with that many parameters you might want to put some of them in a hash (like your attrs hash) to remove the parameter ordering dependency.
2. Use full names for variables etc.. prog --> program; v --> value etc. Increases readability by a lot. Code is read many more times than written.
3. I would extract the code in the if statement into its own method so that you could put it into one line, kind of:  value = some_method(prog, attrs) if value.nil?  Or now that I think of it. You just try to create a value, in the if and in the line before. That could be a method of its own.
4. Value seems to be tied to a specific program. Why not give program a method like create_value:

def create_value(attrs, new_value = nil)
# all the magic happens here
end


Hope that helps :-)

-
Thanks... I just cross posted here per the suggestion over on SO. – pitosalas Nov 10 '12 at 22:33

I'd write:

def self.find_or_create_value(prog_id, part_id, round_id, quest_id, new_value = nil)
program = Program.find(prog_id)
value_attrs = {participant_id: part_id, round_id: round_id, question_id: quest_id}
value = program.values.where(value_attrs).first || program.values.new(value_attrs)
value.value = new_value
value.save
value
end


Note that you lose the exit code of save but in your question is not clear how this should be handled.

-