[PHP-DEV] Inconsistencies between parameter number and index when reflecting a method/function

Mailing Lists php.internals
1

Hi All,

I’ve been working on a PR that introduces ReflectionFunctionAbstract::getParameter() and ReflectionFunctionAbstract::hasParameter(), to fall more inline with the other method sets we have, as well as just generally making peoples lives easier.

The PR is here: https://github.com/php/php-src/pull/10431

These methods accept an integer to retrieve a parameter by its position, or a string to retrieve by its name. So far, I have built this so that if you required the first parameter, it’s parameter 0. I treat it this way because the only other place where we deal with parameter indexes, is ReflectionFunctionAbstract::getParameters() which returns the parameters zero-indexed.

The question that is holding this PR back is should these methods be 1 indexed, so that the provided position is consistent with the error messages, or how a person would typically count, or should they be 0 indexed to remain consistent with the existing API.

Girgias has asked that I pause the PR until we can have a discussion on this mailing list about how to approach it, so I’m looking for feedback on this.

Best Regards,

Ollie Read

2

On Thu, May 2, 2024 at 2:51 PM Ollie Read php@ollie.codes wrote:

Hi All,

I’ve been working on a PR that introduces ReflectionFunctionAbstract::getParameter() and ReflectionFunctionAbstract::hasParameter(), to fall more inline with the other method sets we have, as well as just generally making peoples lives easier.

The PR is here: https://github.com/php/php-src/pull/10431

These methods accept an integer to retrieve a parameter by its position, or a string to retrieve by its name. So far, I have built this so that if you required the first parameter, it’s parameter 0. I treat it this way because the only other place where we deal with parameter indexes, is ReflectionFunctionAbstract::getParameters() which returns the parameters zero-indexed.

The question that is holding this PR back is should these methods be 1 indexed, so that the provided position is consistent with the error messages, or how a person would typically count, or should they be 0 indexed to remain consistent with the existing API.

PHP being a mostly zero indexed language I would say it should be getParamter(0) to get the parameter #1 (referred to as 1 in error messages).

Girgias has asked that I pause the PR until we can have a discussion on this mailing list about how to approach it, so I’m looking for feedback on this.

Best Regards,

Ollie Read

3

On 2 May 2024 13:48:36 BST, Ollie Read <php@ollie.codes> wrote:

These methods accept an integer to retrieve a parameter by its position, or a string to retrieve by its name. So far, I have built this so that if you required the first parameter, it's parameter 0. I treat it this way because the only other place where we deal with parameter indexes, is ReflectionFunctionAbstract::getParameters() which returns the parameters zero-indexed.

The question that is holding this PR back is should these methods be 1 indexed, so that the provided position is consistent with the error messages, or how a person would typically count, or should they be 0 indexed to remain consistent with the existing API.

0-indexed, as that's what PHP does everywhere else.

cheers
Derick

4

On Thu, May 2, 2024 at 10:22 PM Benjamin Außenhofer <kontakt@beberlei.de> wrote:

On Thu, May 2, 2024 at 2:51 PM Ollie Read php@ollie.codes wrote:

Hi All,

I’ve been working on a PR that introduces ReflectionFunctionAbstract::getParameter() and ReflectionFunctionAbstract::hasParameter(), to fall more inline with the other method sets we have, as well as just generally making peoples lives easier.

The PR is here: https://github.com/php/php-src/pull/10431

These methods accept an integer to retrieve a parameter by its position, or a string to retrieve by its name. So far, I have built this so that if you required the first parameter, it’s parameter 0. I treat it this way because the only other place where we deal with parameter indexes, is ReflectionFunctionAbstract::getParameters() which returns the parameters zero-indexed.

The question that is holding this PR back is should these methods be 1 indexed, so that the provided position is consistent with the error messages, or how a person would typically count, or should they be 0 indexed to remain consistent with the existing API.

PHP being a mostly zero indexed language I would say it should be getParamter(0) to get the parameter #1 (referred to as 1 in error messages).

Girgias has asked that I pause the PR until we can have a discussion on this mailing list about how to approach it, so I’m looking for feedback on this.

Best Regards,

Ollie Read

Parameter zero is the first parameter. Both are correct, we access memory from the start (0), but as humans, we count total items. You never say I have zero goats and really mean that you have one goat. However, if you wanted to start at the beginning of a line of goats, you’d start at position zero to get the first goat.

Robert Landers
Software Engineer
Utrecht NL

5

On Thursday, 2 May 2024 at 21:33, Derick Rethans <derick@php.net> wrote:

On 2 May 2024 13:48:36 BST, Ollie Read php@ollie.codes wrote:

> These methods accept an integer to retrieve a parameter by its position, or a string to retrieve by its name. So far, I have built this so that if you required the first parameter, it's parameter 0. I treat it this way because the only other place where we deal with parameter indexes, is ReflectionFunctionAbstract::getParameters() which returns the parameters zero-indexed.
>
> The question that is holding this PR back is should these methods be 1 indexed, so that the provided position is consistent with the error messages, or how a person would typically count, or should they be 0 indexed to remain consistent with the existing API.

0-indexed, as that's what PHP does everywhere else.

cheers
Derick

Well not really, if you have an error (TypeError or ValueError) which indicate what parameter is the problem, it will be 1-indexed.

Which, for me, makes it more logical to have it 1-indexed.
If the ReflectionFunctionAbstract::getParameters() API did not exist, this is what I would have pushed for.

Moreover, PHP already has a 1-indexed and 0-indexed discrepancy with the ob_get_level() and ob_get_status() functions.

In the end, I don't really care what we choose, but this just needed clarification from internals on how to proceed.

Best regards,

Gina P. Banyard

6

On Fri, May 3, 2024 at 4:01 AM Gina P. Banyard <internals@gpb.moe> wrote:

On Thursday, 2 May 2024 at 21:33, Derick Rethans <derick@php.net> wrote:

> On 2 May 2024 13:48:36 BST, Ollie Read php@ollie.codes wrote:
>
> > These methods accept an integer to retrieve a parameter by its position, or a string to retrieve by its name. So far, I have built this so that if you required the first parameter, it's parameter 0. I treat it this way because the only other place where we deal with parameter indexes, is ReflectionFunctionAbstract::getParameters() which returns the parameters zero-indexed.
> >
> > The question that is holding this PR back is should these methods be 1 indexed, so that the provided position is consistent with the error messages, or how a person would typically count, or should they be 0 indexed to remain consistent with the existing API.
>
>
> 0-indexed, as that's what PHP does everywhere else.
>
> cheers
> Derick

Well not really, if you have an error (TypeError or ValueError) which indicate what parameter is the problem, it will be 1-indexed.

Which, for me, makes it more logical to have it 1-indexed.
If the ReflectionFunctionAbstract::getParameters() API did not exist, this is what I would have pushed for.

Moreover, PHP already has a 1-indexed and 0-indexed discrepancy with the ob_get_level() and ob_get_status() functions.

In the end, I don't really care what we choose, but this just needed clarification from internals on how to proceed.

Best regards,

Gina P. Banyard

There's no discrepancy with ob_get_level and ob_get_status.
ob_get_level starts at 0 (no ob) and ob_get_status is count(0) with
no_ob.

count(ob_get_status(true)) === ob_get_level()

Robert Landers
Software Engineer
Utrecht NL

7

Le 2 mai 2024 à 14:48, Ollie Read php@ollie.codes a écrit :

The question that is holding this PR back is should these methods be 1 indexed, so that the provided position is consistent with the error messages, or how a person would typically count, or should they be 0 indexed to remain consistent with the existing API.

It ought to be consistent with the existing reciprocal method ReflectionParameter::getPosition(), which returns 0 for parameter #1.

—Claude

8

On Fri, May 3, 2024 at 9:28 AM Lynn <kjarli@gmail.com> wrote:

The parameter number always confuses me because parameter 4 is actually the 4th, not the 3rd like I’d expect if I look at the indexes when ...$parameters.

I wish we had a proper system so I could edit my message instead of bothering everyone with an additional reply, I meant to say index 3, not the 3rd parameter.

9

On Thu, May 2, 2024 at 10:21 PM Benjamin Außenhofer <kontakt@beberlei.de> wrote:

On Thu, May 2, 2024 at 2:51 PM Ollie Read php@ollie.codes wrote:

Hi All,

I’ve been working on a PR that introduces ReflectionFunctionAbstract::getParameter() and ReflectionFunctionAbstract::hasParameter(), to fall more inline with the other method sets we have, as well as just generally making peoples lives easier.

The PR is here: https://github.com/php/php-src/pull/10431

These methods accept an integer to retrieve a parameter by its position, or a string to retrieve by its name. So far, I have built this so that if you required the first parameter, it’s parameter 0. I treat it this way because the only other place where we deal with parameter indexes, is ReflectionFunctionAbstract::getParameters() which returns the parameters zero-indexed.

The question that is holding this PR back is should these methods be 1 indexed, so that the provided position is consistent with the error messages, or how a person would typically count, or should they be 0 indexed to remain consistent with the existing API.

PHP being a mostly zero indexed language I would say it should be getParamter(0) to get the parameter #1 (referred to as 1 in error messages).

Girgias has asked that I pause the PR until we can have a discussion on this mailing list about how to approach it, so I’m looking for feedback on this.

Best Regards,

Ollie Read

The parameter number always confuses me because parameter 4 is actually the 4th, not the 3rd like I’d expect if I look at the indexes when ...$parameters. Whenever I get an error about parameters I always have to triple check and it costs me extra time to verify the number. I wouldn’t mind having this number be consistent with the array indexes instead. I write Lua on a regular basis and everything starts with 1, but it’s consistent in the behavior so I never have to think twice.

10

On Fri, May 3, 2024, at 1:45 AM, Gina P. Banyard wrote:

On Thursday, 2 May 2024 at 21:33, Derick Rethans <derick@php.net> wrote:

On 2 May 2024 13:48:36 BST, Ollie Read php@ollie.codes wrote:

These methods accept an integer to retrieve a parameter by its position, or a string to retrieve by its name. So far, I have built this so that if you required the first parameter, it’s parameter 0. I treat it this way because the only other place where we deal with parameter indexes, is ReflectionFunctionAbstract::getParameters() which returns the parameters zero-indexed.

The question that is holding this PR back is should these methods be 1 indexed, so that the provided position is consistent with the error messages, or how a person would typically count, or should they be 0 indexed to remain consistent with the existing API.

0-indexed, as that’s what PHP does everywhere else.

cheers

Derick

Well not really, if you have an error (TypeError or ValueError) which indicate what parameter is the problem, it will be 1-indexed.

Which, for me, makes it more logical to have it 1-indexed.

If the ReflectionFunctionAbstract::getParameters() API did not exist, this is what I would have pushed for.

Moreover, PHP already has a 1-indexed and 0-indexed discrepancy with the ob_get_level() and ob_get_status() functions.

In the end, I don’t really care what we choose, but this just needed clarification from internals on how to proceed.

Best regards,

Gina P. Banyard

It looks like the consensus seems to be 0-indexed, but I’m happy to allow it a bit more time to get a few more opinions if you’d prefer.

I do totally understand what you’re saying, though. I think people are typically already capable of differentiation. We know that the first entry in a list array, so entry 1, is index 0. Not to mention the way getPosition() and getParameters() works.

Best Regards,

Ollie Read

11

On Fri, 3 May 2024, Gina P. Banyard wrote:

On Thursday, 2 May 2024 at 21:33, Derick Rethans <derick@php.net> wrote:

> On 2 May 2024 13:48:36 BST, Ollie Read php@ollie.codes wrote:
>
> > These methods accept an integer to retrieve a parameter by its
> > position, or a string to retrieve by its name. So far, I have
> > built this so that if you required the first parameter, it's
> > parameter 0. I treat it this way because the only other place
> > where we deal with parameter indexes, is
> > ReflectionFunctionAbstract::getParameters() which returns the
> > parameters zero-indexed.
> >
> > The question that is holding this PR back is should these methods
> > be 1 indexed, so that the provided position is consistent with the
> > error messages, or how a person would typically count, or should
> > they be 0 indexed to remain consistent with the existing API.
>
> 0-indexed, as that's what PHP does everywhere else.

Well not really, if you have an error (TypeError or ValueError) which
indicate what parameter is the problem, it will be 1-indexed.

Which *API* in PHP is 1-indexed?

cheers,
Derick

12

On Fri, 3 May 2024 at 17:49, Derick Rethans <derick@php.net> wrote:

Which *API* in PHP is 1-indexed?

cheers,
Derick

Certainly isn't normal but found 1 (and only 1!):

$stmt = $pdo->prepare('SELECT name FROM users WHERE id = ?');
$stmt->bindParam(1, $id);

13

Hi,

Il 03/05/2024 17:53, Hans Henrik Bergan ha scritto:

On Fri, 3 May 2024 at 17:49, Derick Rethans <derick@php.net> wrote:

Which *API* in PHP is 1-indexed?

cheers,
Derick

Certainly isn't normal but found 1 (and only 1!):

$stmt = $pdo->prepare('SELECT name FROM users WHERE id = ?');
$stmt->bindParam(1, $id);

I suppose numbering is derived from external (database) standards. E.g. Postgres uses $1, $2, etc. as parameter placeholders.

Also it is in turn is inconsistent with:

$stmt = $pdo->prepare('SELECT name FROM users WHERE id = ?');
$stmt->execute([$id]);

That said, 0-indexed for the reflection PR seems the best choice to me too.

Cheers
--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

14

On Friday, 3 May 2024 at 16:47, Derick Rethans <derick@php.net> wrote:

On Fri, 3 May 2024, Gina P. Banyard wrote:

> On Thursday, 2 May 2024 at 21:33, Derick Rethans derick@php.net wrote:
>
> > On 2 May 2024 13:48:36 BST, Ollie Read php@ollie.codes wrote:
> >
> > > These methods accept an integer to retrieve a parameter by its
> > > position, or a string to retrieve by its name. So far, I have
> > > built this so that if you required the first parameter, it's
> > > parameter 0. I treat it this way because the only other place
> > > where we deal with parameter indexes, is
> > > ReflectionFunctionAbstract::getParameters() which returns the
> > > parameters zero-indexed.
> > >
> > > The question that is holding this PR back is should these methods
> > > be 1 indexed, so that the provided position is consistent with the
> > > error messages, or how a person would typically count, or should
> > > they be 0 indexed to remain consistent with the existing API.
> >
> > 0-indexed, as that's what PHP does everywhere else.
>
> Well not really, if you have an error (TypeError or ValueError) which
> indicate what parameter is the problem, it will be 1-indexed.

Which API in PHP is 1-indexed?

cheers,
Derick

I have given the 1-index API which is ob_get_level() in my email reply, so I'm confused by the question here.
ob_get_level() starts at 1 for the 1st level and increases from there. If the function returns 0 then output buffering is disabled.

However, if you retrieve the full statuses of output handlers via ob_get_status() it returns a 0-index array.
So to get the correspondence between the output buffer from ob_get_level() and ob_get_status() you need to do +/- 1 with the indexes/levels.

I am just pointing out that such a difference in APIs already exists *within* PHP.

Like I said earlier, I don't frankly care *what* the consensus is, but considering it was split *within* the reviewers of the PR it needed to be brought to the attention of internals.
I find 0-indexing in this case utterly confusing, but that's possibly just me.
And if most people are in favour of 0-indexing, then let it be 0-indexing.

Best regards,

Gina P. Banyard

15

On Thursday, 2 May 2024 at 13:48, Ollie Read <php@ollie.codes> wrote:

Hi All,

I've been working on a PR that introduces ReflectionFunctionAbstract::getParameter() and ReflectionFunctionAbstract::hasParameter(), to fall more inline with the other method sets we have, as well as just generally making peoples lives easier.

The PR is here: https://github.com/php/php-src/pull/10431

These methods accept an integer to retrieve a parameter by its position, or a string to retrieve by its name. So far, I have built this so that if you required the first parameter, it's parameter 0. I treat it this way because the only other place where we deal with parameter indexes, is ReflectionFunctionAbstract::getParameters() which returns the parameters zero-indexed.

The question that is holding this PR back is should these methods be 1 indexed, so that the provided position is consistent with the error messages, or how a person would typically count, or should they be 0 indexed to remain consistent with the existing API.

Girgias has asked that I pause the PR until we can have a discussion on this mailing list about how to approach it, so I'm looking for feedback on this

I am not sure why I had not thought about checking this earlier, but ReflectionParameter has a getPosition method [1], where the first parameter of a function/method is 0.
So the _best_ argument for having it 0 indexed based is to be consistent with the existing semantics.

So I am fine with having it 0 indexed.

Best regards,
Gina P. Banyard

[1] PHP: ReflectionParameter::getPosition - Manual