Applying limit and offset to an array

Question

I'm refactoring an old gateway and came across the following method:

def apply_limit_and_offset(options)
 limit = options[:limit]
 offset = options[:offset]
 @result = if offset
 @result.drop offset
 else
 @result
 end
 @result = if limit
 @result.first limit
 else
 @result
 end
 @result
end

@result is an Array of objects.

Any suggestions on how to refactor this? It works just fine, but multiple if statements make me feel gross.

Answer 1

Some notes:

• Be consistent with the use of parens.

• Use ternary expressions (?) to reduce short conditionals.

• If you are using Ruby>= 2.0, check named keywords.

• The resulting expression of an assignment is its value, so you can remove the last @result

• What looks gross to me are those in-place updates of @result, but this goes beyond the scope of the question.

With Ruby>= 2.0, I'd write:

def apply_limit_and_offset(limit: nil, offset: nil)
 @result = offset ? @result.drop(offset) : @result
 @result = limit ? @result.first(limit) : @result
end

• dax: named keywords are really nice, i had no idea about them. thanks!
• Evan Bechtol: That's some pretty code; nice and elegant. +1
• Cary Swoveland: You could also write the first line, @result.shift(offset) if offset.

Answer 2

Your operation is basically equivalent to Array#slice, so that is how I would write it.

def apply_limit_and_offset(options)
 @result = @result.slice(options[:offset] || 0,
 options[:limit] || @result.length) || []
end

#slice appears to tolerate an options[:limit] that goes beyond the end of the array. However, if options[:offset] is out of range, then #slice returns nil, so we need to add || [] at the end to make it an empty array.

• Cary Swoveland: Very nice, but where's your usual thorough explanation? Why does this work for various case? For example, what if offset >= @result.size or 0 <= offset < @result.size and offset + limit >= @result.size?
• 200_success: @CarySwoveland Indeed, the original version was all wrong.