Reputation: 11362
Is there a better way to find if the current element of a collection is the last element?
@oranges = Orange.all
@oranges.each_with_index do |o, current_index|
puts @oranges.size == (current_index + 1) ? "Last element!" : "Get next element"
end
OR
@oranges.each do |o|
puts @oranges.last == o ? "Last element!" : "Get next element"
end
Upvotes: 1
Views: 1601
Reputation: 64177
Either looks fine, however if you were seriously considered about the perforamnce, I ran some benchmarks:
require 'benchmark'
a = (1..1000).to_a
def first(a)
a.each_with_index do |o,i|
a.size == (i + 1)
end
end
def first_cached(a)
a_size = a.size
a.each_with_index do |o,i|
a_size == (i + 1)
end
end
def second(a)
a.each do |e|
a.last == e
end
end
def second_cached(a)
a_last = a.last
a.each do |e|
a_last == e
end
end
Benchmark.bm(7) do |x|
x.report("first") {10000.times {first(a)}}
x.report("first_cached") {10000.times{first_cached(a)}}
x.report("second") {10000.times{second(a)}}
x.report("second_cached") {10000.times{second_cached(a)}}
end
which returned:
user system total real
first 2.020000 0.010000 2.030000 ( 2.024102)
first_cached 1.930000 0.000000 1.930000 ( 1.947230)
second 1.920000 0.010000 1.930000 ( 1.922338)
second_cached 1.350000 0.000000 1.350000 ( 1.352786)
So the second version, with a cached size yielded better results... however if these micro performances don't matter, it shouldn't be a problem.
Upvotes: 3
Reputation: 47971
Does the first solution even work? o
is an orange and it doesn't have the size
method.
You can instead do :
@oranges.each_with_index do |o, current_index|
puts current_index == @oranges.size - 1 ? "Last element!" : "Get next element"
end
Apart from that both of the approaches are fine.
Upvotes: 1