Refactor to avoid cope repetition in loop with condition checking

Question

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)

Answer 1

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

Answer 2

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