I dislike it as there can't be any efficient way to implement it. If you want to do #last, use an Indexable

Sometimes we just need to take the last element - is it more efficient to use an Indexable vs a #last implementation like mine? If no, it worth it.

This makes no sense. Besides the fact that this can't be implemented in an efficient way, an Enumerable does not necessarily even have a last element (e.g. cyclic or otherwise infinite iterators).

Indexable defines a #last method, and that's where it belongs.

I don't see the significance for #5760. Hash#last could be implemented without inheriting it from Enumerable. But if Hash had a #last method, it should probably also be an Indexable.

I agree with all the above. I vote to close. This already exists on Indexable where it belongs.

Fine, I close this

I would like also to post benchmarks:

Benchmark.ips do |x|
  x.report("last with Enumerable") do
    (0..9).last
  end

  x.report("last with Indexable") do
    (0..9).to_a[-1]
  end
end

Results with (0..9):

last with Enumerable 584.85M (  1.71ns) (± 2.91%)       fastest
 last with Indexable   8.66M (115.46ns) (± 3.30%) 67.53× slower

Results with (0..999):

last with Enumerable 579.68M (  1.73ns) (± 4.95%)         fastest
 last with Indexable 194.34k (  5.15µs) (± 7.41%) 2982.84× slower

Maybe I've done it wrong, but the performance difference is significant.

EDIT: I've switched Indexable and Enumerable, my bad

Well, creating the array in the enumerable example is unnecessary and probably adds quite a bit of overhead because of heap allocations. You should just do range.each.last with your implementation of Enumerable#last.

But there seems to be an error with Range.each as the ItemIterator loops around - so that's an unintended example of an enumerable that does not have a last element.

@straight-shoota I've switched Indexable and Enumerable in my code sample - this not change the results by the way

the correct code is to do ARRAY = (0..9).to_a and then test ARRAY.each.last vs ARRAY.last.

The point is to avoid indexing to obtain the last value from an enumerable. Creating an array from an enumerable means indexing, and a performance penality..

Also, having ten numbers won't be a big difference in any case. Try with a million entries or so (and as other stated, do the allocation outside the benchmarks - those are not interesting here).

@j8r I meant for the benchmark, not for actual usage.

Thanks for the feedbacks:

module Enumerable(T)
  def last
    last = uninitialized T

    each { |e| last = e }
    last
  end
end

ENUMERABLE = (0..99999)
ARRAY = ENUMERABLE.to_a

Benchmark.ips do |x|
  x.report("last with Enumerable") do
    ENUMERABLE.last
  end

  x.report("last with Indexable") do
    ARRAY.last
  end
end

With found:

last with Enumerable   8.51k (117.48µs) (± 3.33%) 57426.78× slower
 last with Indexable  488.8M (  2.05ns) (± 4.73%)          fastest

Without found:

last with Enumerable 503.43M (  1.99ns) (± 2.33%)       fastest
 last with Indexable 335.24M (  2.98ns) (± 4.35%)  1.50× slower

with #last? (that require found, unlike #last)

last with Enumerable 501.56M (  1.99ns) (± 2.67%)       fastest
 last with Indexable 331.52M (  3.02ns) (± 4.34%)  1.51× slower

The second values can't possibly be anywhere near realistic. It should be pretty much the same as with found. Please note that your sample code won't compile. And just leaving out found is completely wrong as it can lead to uninitialized values leaking out. You should set last = nil instead of uninitalized.

I was very confused with the above benchmark results. I thought it is impossible that Enumerable#last is faster than Indexable#last, but it also happens on my computer, and I think I've found the reason.

the results of the above benchmark on my computer:

last with Enumerable 523.01M (  1.91ns) (± 9.59%)       fastest
 last with Indexable 358.11M (  2.79ns) (±10.78%)  1.46× slower

but when running it without --release:

Warning: benchmarking without the `--release` flag won't yield useful results
last with Enumerable   4.33k (230.87µs) (± 7.82%) 24324.43× slower
 last with Indexable 105.36M (  9.49ns) (±11.12%)          fastest

In conclusion, the reason why Enumerable#last is faster than Indexable#last is 🎉 optimization 🎉

looks like LLVM optimizes out the Enumerable#last result entirely. This is why we should port JMH's blackhole.

I have not yet seen any code for a valid benchmark with that reproducible result.

The last example by @j8r is crap. It reuses the Range as iterator, which means only the first call to ENUMERABLE.last will walk through all elements, in the following runs the each block will never be called because the iterator is at the end. Because of last = uninitialized T it still returns a valid Int32 (and in my test it is even the last element of the iterator). So yeah, maybe LLVM pulls some crazy optimization but this only possible because the example code is faulty.

A proper benchmark needs to either use fresh iterator for every run or reset it in between.

The following code runs in release mode without any strange optimizations and yields a plausible result:

require "benchmark"

module Enumerable(T)
  def last
    last = uninitialized T
    found = false

    each { |e| last = e; found = true }

    raise EmptyError.new unless found
    last
  end
end

ARRAY = (0..9999).to_a

Benchmark.ips do |bm|
  bm.report("Enumerable#last") do
    ARRAY.each.last
  end
  bm.report("Indexable#last") do
    # ARRAY.each
    ARRAY.last
  end
end

Of course, this is not entirely accurate, because the enumerable test also initializes a new iterator on each run (that's the 32 B heap allocation). But that's completely negligible compared to looping through thousands of array items.

Enumerable#last  32.34k ( 30.92µs) (± 9.67%)  32 B/op  3170.13× slower
 Indexable#last 102.52M (  9.75ns) (±11.42%)   0 B/op          fastest

To equalize this, I have added ARRAY.each to the indexable test as well (commented out above) and while the impact seems to be one order of magnitude, the difference in performance is still very significant, even for smaller array sizes:

# array size 10_000:
Enumerable#last  32.63k ( 30.64µs) (±10.11%)  32 B/op  404.31× slower
 Indexable#last  13.19M ( 75.79ns) (±19.81%)  32 B/op         fastest
# array size 10:
Enumerable#last   7.52M (133.05ns) (±10.07%)  32 B/op   1.56× slower
 Indexable#last  11.72M (  85.3ns) (±11.03%)  32 B/op        fastest

@straight-shoota

The last example by @j8r is crap. It reuses the Range as iterator, which means only the first call to ENUMERABLE.last will walk through all elements, in the following runs the each block will never be called because the iterator is at the end. Because of last = uninitialized T it still returns a valid Int32 (and in my test it is even the last element of the iterator).

But it's not crap, each is still working as expect because it is Range#each but not Iterator#each, and Range doesn't include Iterator.

https://carc.in/#/r/3rfu

I'm pretty sure there was different code before.

But anyway, it makes no sense to benchmark code that can leak uninitialized values. It's certainly not usable so there is no point in performance testing it.