Calling Sequence->get() with an index of -1 doesn't work in 4.8+

[Jamesking56]

Bug Report

Information	Description
Version	4.8.0
PHP version	7.2.19
OS Platform	Linux (Ubuntu 18.04)

Summary

According to the documentation, the get() method supports passing in a negative index from version 4.8 and above. When I do this however, I get InvalidIndex exception thrown with the message:

-1 is an invalid index in the current sequence

Standalone code, or other way to reproduce the problem

Create any Sequence (empty or not) and call ->get(-1).

Expected result

I'm expecting that the return value is the latest Period item in the Sequence as shown in the documentation:

https://period.thephpleague.com/4.0/sequence/collection/#sequenceget

Actual result

InvalidIndex exception is thrown citing that -1 is an invalid index.

[nyamsprod]

@Jamesking56 thanks for using the library. Could you provide a snippet please as this feature is covered by tests I would need to see a concrete example in order to understand the origin of the issue if there's one.

thanks in advance.

[Jamesking56]

Well, its the smallest concrete example known to man really, I don't think your tests actually cover negative offsets:

$sequence = new Sequence(
    new Period(
        new \DateTimeImmutable('yesterday'),
        new \DateTimeImmutable('now')
    )
);

$sequence->get(-1); // Exception instead of expected period.

[Jamesking56]

Same with an empty Sequence, I think it should return null in this instance:

$sequence = new Sequence();

$sequence->get(-1); // Exception instead of expected period.

[nyamsprod]

$sequence = new Sequence();

$sequence->get(-1); // Exception instead of expected period.

This is correct with an empty sequence get should definitely throw ...

The first example is debatable but I can see where your reasoning comes from ... I'll see if I'll fix it or not it needs thinking first.

[Jamesking56]

Looks like its just the Sequence with a single item that needs fixing then, as this correct logic works when there is 2 or more items.

[Jamesking56]

Thinking further about this, I think it would be a lot nicer if there was a latest method on Sequence that returned the latest Period or null if there isn't one. No exceptions thrown.

[nyamsprod]

Thinking further about this, I think it would be a lot nicer if there was a latest method on Sequence that returned the latest Period or null if there isn't one. No exceptions thrown.

Just use the Sequence::isEmpty method for that

[nyamsprod]

the issue is fixed ... it will be released in a patch version in the upcoming days

[nyamsprod]

the 4.8.1 release it out with the fix.

[Jamesking56]

Thanks @nyamsprod