URI::getSegment() returns non-existent segment

[kenjis]

CodeIgniter version: 4.3.1

In this sample, there are three URI segments: 1:users, 2:15, 3:profile
But you can get the 4th segment $uri->getSegment(4, 'bar') without Exception.
But $uri->getSegment(5, 'baz') will throw Exception.

// URI = http://example.com/users/15/profile

// will print 'profile'
echo $uri->getSegment(3, 'foo');
// will print 'bar'
echo $uri->getSegment(4, 'bar');
// will throw an exception
echo $uri->getSegment(5, 'baz');
// will print 'baz'
echo $uri->setSilent()->getSegment(5, 'baz');
// will print '' (empty string)
echo $uri->setSilent()->getSegment(5);

https://codeigniter4.github.io/CodeIgniter4/libraries/uri.html#uri-segments

Related: #7233

[kenjis]

This was already reported in #2962

[kenjis]

The current behavior is difficult to understand and I still consider it a bug.

If there is no segment, an exception should be thrown.
If you don't want an exception, you can use setSilent().

[michalsn]

As much as I agree this is strange behavior, the n+1 implementation to not throw an exception was intended from the beginning (from what I understand). However, there was no setSilent() method from the beginning, so then it had much more sense.

Changing this now for a minor version release may be problematic for many existing projects.

[kenjis]

What is the intention? Why only n+1 is okay?

[michalsn]

That was more convenient, I guess.

n+1 means an additional, optional parameter in the URL that we can check without worrying about throwing an exception if it's not there. It can be for pagination, filtering, etc., a standard approach from my perspective.

To be fair - for me throwing an exception by default was always a bad idea. In this particular case, I would replace it with a null value.

Anyway, I think that changing this behavior may be potentially harmful.

[kenjis]

To be fair - for me throwing an exception by default was always a bad idea. In this particular case, I would replace it with a null value.

I don't know why these changes were made in v4.0.
https://github.com/bcit-ci/CodeIgniter/blob/45576ef6e62b5ff59da67f1697ee8c57809c7719/system/core/URI.php#L345-L348

[kenjis]

This is the first commit for segment():
09b8e8be#diff-3246cb97b91e8f9534e108c0f9a1c8de5e28af7d5983575a02c328823fd1625fR279

It seems to me that accepting n+1 is just a bug.
I think $number -= 1; should be after the if.

[kenjis]

@michalsn Do you think we should accept n+1 in v5?

[michalsn]

If this is a bug or not - I can't answer this because I don't know. For me, the idea of throwing an exception here is wrong. It's like we would throw an exception by default when there is no GET parameter.

Do you think we should accept n+1 in v5?

No.

[lonnieezell]

I believe that code was copied straight over from v3, IIRC. If that's' true, it's how it has always been and then you're torn between how the developer has to handle it - check for null or a try/catch block. Is it an exceptional moment in the program? I'm not sure it always is, but I have no strong feelings either way for this. But there's possible a lot of code out there relying on it currently. Unnecessary BC break, if you ask me.

[kenjis]

I changed my opinion.

n+1 means an additional, optional parameter in the URL that we can check without worrying about throwing an exception if it's not there. It can be for pagination, filtering, etc., a standard approach from my perspective.

These use cases are likely, and I think getting n+1 segment is not exceptional.
Setting n+1 is acceptable, so accept to get n+1 is consistent.