Refactor to avoid cope repetition in loop with condition checking

I have snippet that really look ugly but I don't know how to refactor it:

if owner
samples.each do |sample|
sample_name = get_name(sample) # logical to get correct name
serialized_sample = SampleSerializer::Level10.new(sample, 10).serializable_hash
groups[sample_name] = groups[sample_name] == nil
? [].push(serialized_sample)
: groups[sample_name].push(serialized_sample)
end
else
samples.each do |sample|
sample_name = get_name(sample) # logical to get correct name
serialized_sample = ElementPermissionProxy.new(current_user, sample, user_ids).serialized
groups[sample_name] = groups[sample_name] == nil
? [].push(serialized_sample)
: groups[sample_name].push(serialized_sample)
end
end


As you can see the code block inside if .. else is mostly the same except I need to use 2 difference class on each case (SampleSerializer and ElementPermissionProxy).

I can change into:

samples.each do |sample|
sample_name = get_name(sample) # logical to get correct name
serialized_sample = owner
? SampleSerializer::Level10.new(sample, 10).serializable_hash
: ElementPermissionProxy.new(current_user, sample, user_ids).serialized

groups[sample_name] = groups[sample_name] == nil
? [].push(serialized_sample)
: groups[sample_name].push(serialized_sample)
end


However, the samples contains a few thousands of element, so that I have check the condition with every sample element. And I don't like this way of optimization.

In addition, I would really appreciate if someone help me with this block also:

  groups[sample_name] = groups[sample_name] == nil
? [].push(serialized_sample)
: groups[sample_name].push(serialized_sample)

• As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. – Jamal Aug 19 '16 at 10:35

Have you considered using a Proc or a Lambda?

You could do the check once, before the loop, to decide the function to call to serialize a sample.

In pseudo Ruby, you need to work out the number of the arguments, I think — I'm not a Ruby programmer.

serializer = owner
? lambda { return SampleSerializer::Level10.new(sample, 10).serializable_hash }
: lambda { return ElementPermissionProxy.new(current_user, sample, user_ids).serialized }

samples.each do |sample|
sample_name = get_name(sample) # logical to get correct name
serialized_sample = serializer.call(sample, current_user, user_ids)
groups[sample_name] = groups[sample_name] == nil
? [].push(serialized_sample)
: groups[sample_name].push(serialized_sample)
end


Thanks for SQB's answer. I've refactored my code using lambda (other variables within lambda such as: current_user and user_ids belong to current scope).

The nil checking of groups[sample_name] array is also be changed using || operator like below. The code is now much more better now:

sample_serializer_selector =
if own_collection
lambda { |s| SampleSerializer::Level10.new(s, 10).serializable_hash }
else
lambda { |s| ElementPermissionProxy.new(current_user, s, user_ids).serialized }
end

samples.each do |sample|
sample_name = get_name(sample)
serialized_sample = sample_serializer_selector.call(sample)
groups[sample_name] = (groups[sample_name] || []).push(serialized_sample)
end