[RFC] skipping optional parameters

Ryan McCue

14 years ago

Stas Malyshev wrote:

> Basically, it allows you to do this:
>
> create_query("deleted=0", "name",,, /*report_errors*/ true);
>

I'm a huge fan of this idea, for all the reasons listed on the RFC and
one additional one: it brings normal functions in line with the syntax
for list() - We can skip parameters in list() using the same syntax, so
to my eye, this brings consistency.

(Of course, list() is a construct, not a function, but considering how
similar the syntax is, I think this makes sense.)

Jordi Boggiano

14 years ago

On 18.04.2012 00:54, Stas Malyshev wrote:

> One of the annoying things I've encountered in working with PHP was
> dealing with functions having long optional parameter lists, especially
> if you need to change only the last one - you have to copy all the
> defaults. Full named params implementation would solve it, probably, but
> before we have that here's an easier solution for part of the problem:
>
> https://wiki.php.net/rfc/skipparams
>
> Basically, it allows you to do this:
>
> create_query("deleted=0", "name",,, /*report_errors*/ true);

Pretty cool especially since it avoids the problem when refactoring code
of knowing whether the use of the default param was intentional or just
as a filler to reach a later parameter.

I do have two remarks though:

- I hope it doesn't mean giving up on named args, because that would
look way cleaner.

- Would it be possible to allow trailing commas in combination to this?
i.e. foo("arg",) would just call foo with "arg" and then the default
value for the second arg if there is one, or just nothing. That would
make multi-line function calls a bit cleaner, and more similar to arrays.

Cheers

--
Jordi Boggiano
@seldaek - http://nelm.io/jordi

Michael Wallner

14 years ago

On 18 April 2012 01:23, Jordi Boggiano <j.boggiano@seld.be> wrote:

> On 18.04.2012 00:54, Stas Malyshev wrote:

>> https://wiki.php.net/rfc/skipparams
>>
>> Basically, it allows you to do this:
>>
>> create_query("deleted=0", "name",,, /*report_errors*/ true);

+1

> - Would it be possible to allow trailing commas in combination to this?
> i.e. foo("arg",) would just call foo with "arg" and then the default
> value for the second arg if there is one, or just nothing. That would
> make multi-line function calls a bit cleaner, and more similar to arrays.

Yet another +1!

--
Regards,
Mike

Richard Lynch

14 years ago

On Tue, April 17, 2012 6:23 pm, Jordi Boggiano wrote:

> On 18.04.2012 00:54, Stas Malyshev wrote:
>> One of the annoying things I've encountered in working with PHP was
>> dealing with functions having long optional parameter lists,
>> especially
>> if you need to change only the last one - you have to copy all the
>> defaults. Full named params implementation would solve it, probably,
>> but
>> before we have that here's an easier solution for part of the
>> problem:
>>
>> https://wiki.php.net/rfc/skipparams
>>
>> Basically, it allows you to do this:
>>
>> create_query("deleted=0", "name",,, /*report_errors*/ true);

It might be worth mentioning in the docs edge cases like strtok.

As you all know, I presume, the optional first param restarts parsing
with a new string.  When there is only one param, it continues parsing
the previous first argument.

As silly as this may seem to you experts, newbies will probably be
confused by the optional argument syntax versus functions that change
the parameter position...

--
brain cancer update:
http://richardlynch.blogspot.com/search/label/brain%20tumor
Donate:
https://www.paypal.com/cgi-bin/webscr?cmd=_s-xclick&hosted_button_id=FS9NLTNEEKWBE

Yasuo Ohgaki

14 years ago

Hi,

2012/4/18 Stas Malyshev <smalyshev@sugarcrm.com>:

> Hi!
>
> One of the annoying things I've encountered in working with PHP was
> dealing with functions having long optional parameter lists, especially
> if you need to change only the last one - you have to copy all the
> defaults. Full named params implementation would solve it, probably, but
> before we have that here's an easier solution for part of the problem:
>
> https://wiki.php.net/rfc/skipparams
>
> Basically, it allows you to do this:
>
> create_query("deleted=0", "name",,, /*report_errors*/ true);
>
> Implementation of it is here:
> https://github.com/smalyshev/php-src/tree/skip_params
> All things described in RFC seem to be working, please tell me if I
> forgot anything. If it's accepted I'll also add tests, etc. of course.

Introducing "undefined" might be good.
Intention becomes clearer and we may do

if ($var === undefined) {
echo '$var is undefined';
}

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Stas Malyshev

14 years ago

Hi!

> Introducing "undefined" might be good.
> Intention becomes clearer and we may do
>
> if ($var === undefined) {
>   echo '$var is undefined';
> }

What is undefined and how $var gets assigned it?

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Yasuo Ohgaki

14 years ago

Hi,

2012/4/18 Stas Malyshev <smalyshev@sugarcrm.com>:

> Hi!
>
>> Introducing "undefined" might be good.
>> Intention becomes clearer and we may do
>>
>> if ($var === undefined) {
>>   echo '$var is undefined';
>> }
>
> What is undefined and how $var gets assigned it?

I'm thinking undefined like JavaScript.

AFAIK, engine does not have undefined state.
It may be too many changes required.

Just skipping would work and I'm not against it.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Yasuo Ohgaki

14 years ago

Hi,

It's undefined, so it does not have to keep track state ;)
But it may require a lot of changes, so I don't matter much.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

2012/4/18 Yasuo Ohgaki <yohgaki@ohgaki.net>:

Stas Malyshev

14 years ago

Hi!

> I'm thinking undefined like JavaScript.

Well, in PHP null is closest, but a bit different. I.e. in JS undefined
a is an error, but a[0] where a is empty is "undefined". In PHP in both
cases you get null and notice, and in both cases you can use isset/empty
to check for it.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Simon J Welsh

14 years ago

On 18/04/2012, at 10:54 AM, Stas Malyshev wrote:

> Hi!
>
> One of the annoying things I've encountered in working with PHP was
> dealing with functions having long optional parameter lists, especially
> if you need to change only the last one - you have to copy all the
> defaults. Full named params implementation would solve it, probably, but
> before we have that here's an easier solution for part of the problem:
>
> https://wiki.php.net/rfc/skipparams
>
> Basically, it allows you to do this:
>
> create_query("deleted=0", "name",,, /*report_errors*/ true);
>
> Implementation of it is here:
> https://github.com/smalyshev/php-src/tree/skip_params
> All things described in RFC seem to be working, please tell me if I
> forgot anything. If it's accepted I'll also add tests, etc. of course.

Hi

I like this proposal. One thing pops out at me though.

Since func_num_args() won't be able to be used to see if an argument is set, could a func_isset_arg() be added? So something like:
if(func_num_args() > 2) $value = func_get_arg(2);
would become:
if(func_isset_arg(2)) $value = func_get_arg(2);
---
Simon Welsh

Yasuo Ohgaki

14 years ago

Hi,

2012/4/18 Simon J Welsh <simon@welsh.co.nz>:

> On 18/04/2012, at 10:54 AM, Stas Malyshev wrote:
>
>> Hi!
>>
>> One of the annoying things I've encountered in working with PHP was
>> dealing with functions having long optional parameter lists, especially
>> if you need to change only the last one - you have to copy all the
>> defaults. Full named params implementation would solve it, probably, but
>> before we have that here's an easier solution for part of the problem:
>>
>> https://wiki.php.net/rfc/skipparams
>>
>> Basically, it allows you to do this:
>>
>> create_query("deleted=0", "name",,, /*report_errors*/ true);
>>
>> Implementation of it is here:
>> https://github.com/smalyshev/php-src/tree/skip_params
>> All things described in RFC seem to be working, please tell me if I
>> forgot anything. If it's accepted I'll also add tests, etc. of course.
>
>
> Hi
>
> I like this proposal. One thing pops out at me though.
>
> Since func_num_args() won't be able to be used to see if an argument is set, could a func_isset_arg() be added? So something like:
> if(func_num_args() > 2) $value = func_get_arg(2);
> would become:
> if(func_isset_arg(2)) $value = func_get_arg(2);

Good point.
For func_get_arg(), having "undefined" is mandatory,
otherwise we cannot tell if it is defined or not.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Stas Malyshev

14 years ago

Hi!

> Since func_num_args() won't be able to be used to see if an argument is set, could a func_isset_arg() be added? So something like:
> if(func_num_args() > 2) $value = func_get_arg(2);
> would become:
> if(func_isset_arg(2)) $value = func_get_arg(2);

I think it's easier to just do func_get_args() and check for isset there
:) You usually don't have too many args so getting all of them is not a
big problem.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Etienne Kneuss

14 years ago

Hi,

On Wed, Apr 18, 2012 at 01:56, Stas Malyshev <smalyshev@sugarcrm.com> wrote:

> Hi!
>
>> Since func_num_args() won't be able to be used to see if an argument is set, could a func_isset_arg() be added? So something like:
>> if(func_num_args() > 2) $value = func_get_arg(2);
>> would become:
>> if(func_isset_arg(2)) $value = func_get_arg(2);
>
> I think it's easier to just do func_get_args() and check for isset there
> :) You usually don't have too many args so getting all of them is not a
> big problem.
>

+1

BTW: the diffs show a lot of WS changes that you should probably fix.

Best,

> --
> Stanislav Malyshev, Software Architect
> SugarCRM: http://www.sugarcrm.com/
> (408)454-6900 ext. 227
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>

--
Etienne Kneuss
http://www.colder.ch

Nikita Popov

14 years ago

On Wed, Apr 18, 2012 at 2:03 AM, Etienne Kneuss <colder@php.net> wrote:

> Hi,
>
>
> On Wed, Apr 18, 2012 at 01:56, Stas Malyshev <smalyshev@sugarcrm.com> wrote:
>> Hi!
>>
>>> Since func_num_args() won't be able to be used to see if an argument is set, could a func_isset_arg() be added? So something like:
>>> if(func_num_args() > 2) $value = func_get_arg(2);
>>> would become:
>>> if(func_isset_arg(2)) $value = func_get_arg(2);
>>
>> I think it's easier to just do func_get_args() and check for isset there
>> :) You usually don't have too many args so getting all of them is not a
>> big problem.
>>
>
> +1
>
> BTW: the diffs show a lot of WS changes that you should probably fix.

On a side note, one can pass ?w=1 to the github diff, so it will show
up without WS changes:
https://github.com/smalyshev/php-src/commit/0f78493d3e96a627caa8258ee9e618301b6b3511?w=1
Makes is a bit easier to spot the relevant parts.

I'm also +1 on the proposal :)

Nikita

Stas Malyshev

14 years ago

Hi!

>> BTW: the diffs show a lot of WS changes that you should probably fix.

That's Eclipse, it loves to strip trailing whitespace... Which btw shows
for some reason we have lots of it in our code.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Yasuo Ohgaki

14 years ago

Hi,

2012/4/18 Stas Malyshev <smalyshev@sugarcrm.com>:

> Hi!
>
>> Since func_num_args() won't be able to be used to see if an argument is set, could a func_isset_arg() be added? So something like:
>> if(func_num_args() > 2) $value = func_get_arg(2);
>> would become:
>> if(func_isset_arg(2)) $value = func_get_arg(2);
>
> I think it's easier to just do func_get_args() and check for isset there
> :) You usually don't have too many args so getting all of them is not a
> big problem.

This works.
I think it's fine with this.

There may be a few codes that have BC issues, but
it's new PHP anyway.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Alan Knowles

14 years ago

On Wednesday, April 18, 2012 06:54 AM, Stas Malyshev wrote:

> Hi!
>
> https://wiki.php.net/rfc/skipparams
>
> Basically, it allows you to do this:
>
> create_query("deleted=0", "name",,, /*report_errors*/ true);
>
> Implementation of it is here:
> https://github.com/smalyshev/php-src/tree/skip_params
> All things described in RFC seem to be working, please tell me if I
> forgot anything. If it's accepted I'll also add tests, etc. of course.
>

Hate to be contrary, it does raise a few questions.

a) it kind of encourages using long lists of arguments (which is not
normally regarded as good practice)

b) the usual recommendation for solving that issue is to accept an array
(fake named arguments)

c) the current situation would syntax error out on ,, as a function call
argument, which is handy if you accidentally typed ,, twice by accident.
if this changes, not only would you loose this syntax error, but it
would do unpredictable things.

For my 2c, does not look like a very good idea..

Regards
Alan

Stas Malyshev

14 years ago

Hi!

> a) it kind of encourages using long lists of arguments (which is not
> normally regarded as good practice)

This ship has sailed long ago, these lists are the reality. And
rewriting all the code to change all function calls is in most cases
completely infeasible.

> c) the current situation would syntax error out on ,, as a function call
> argument, which is handy if you accidentally typed ,, twice by accident.
> if this changes, not only would you loose this syntax error, but it
> would do unpredictable things.

There are many situations where you could type something by accident and
have it change the meaning of the call. For example, if to type:
if($foo); bar(); then bar() would execute always. This does not prevent
us from using ; as statement separator or from using if's.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Adam Jon Richardson

14 years ago

On Tue, Apr 17, 2012 at 6:54 PM, Stas Malyshev <smalyshev@sugarcrm.com>wrote:

> Hi!
>
> One of the annoying things I've encountered in working with PHP was
> dealing with functions having long optional parameter lists, especially
> if you need to change only the last one - you have to copy all the
> defaults. Full named params implementation would solve it, probably, but
> before we have that here's an easier solution for part of the problem:
>
> https://wiki.php.net/rfc/skipparams
>
> Basically, it allows you to do this:
>
> create_query("deleted=0", "name",,, /*report_errors*/ true);
>
> Implementation of it is here:
> https://github.com/smalyshev/php-src/tree/skip_params
> All things described in RFC seem to be working, please tell me if I
> forgot anything. If it's accepted I'll also add tests, etc. of course.

Stas, this is fantastic! Not a voter, but I would be very thankful for this
addition to PHP.

Adam

Alexey Shein

14 years ago

Hi!

My opinion is that solution tries to overcome bad consequences of
legacy code, when it's not feasible to change something without
breakage a lot of code, although the real solution is to refactor
functions with long variable lists (as Uncle Bob says), maybe this
should be noted in documentation of this feature.

Alan nicely highlighted all cons of this feature, but instead of
voting against it I suggest to put that into the docs, so people can
use it wisely and understand consequences.

One question about implementation:
Given we have this function

>   function create_query($where, $order_by, $join_type='', $execute = false, $report_errors = true) {}

and this statement

> On the engine level, it will be implemented by putting NULL in the place where the parameter is passed.

what value $join_type will get on this call?
create_query("deleted=0", "name",,, /*report_errors*/ true);

NULL or empty string? I.e. will optional parameters always get NULLs
or their default values?
Thanks.

--
Regards,
Shein Alexey

Peter Lind

14 years ago

On 18 April 2012 07:56, Alexey Shein <confik@gmail.com> wrote:

*snip*

>
> One question about implementation:
> Given we have this function
>>   function create_query($where, $order_by, $join_type='', $execute = false, $report_errors = true) {}
> and this statement
>> On the engine level, it will be implemented by putting NULL in the place where the parameter is passed.
>
> what value $join_type will get on this call?
> create_query("deleted=0", "name",,, /*report_errors*/ true);
>
> NULL or empty string? I.e. will optional parameters always get NULLs
> or their default values?

It needs to be the default values, otherwise it will be a huge WTF for
developers.

With that in mind: +1 if default values are used when skipping
parameters, -1 if NULL is used when skipping parameters (not that I
have voting power, just showing preferences/support).

Regards
Peter

--
<hype>
WWW: plphp.dk / plind.dk
LinkedIn: plind
BeWelcome/Couchsurfing: Fake51
Twitter: kafe15
</hype>

Stas Malyshev

14 years ago

Hi!

> NULL or empty string? I.e. will optional parameters always get NULLs
> or their default values?

Their default values. That's the whole point of it - if I wanted just
nulls, I could put nulls in the call, but if I want the actual default,
I'd have to find the definition and copy-paste from there.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Alexey Shein

14 years ago

18 апреля 2012 г. 11:40 пользователь Stas Malyshev
<smalyshev@sugarcrm.com> написал:

> Hi!
>
>> NULL or empty string? I.e. will optional parameters always get NULLs
>> or their default values?
>
> Their default values. That's the whole point of it - if I wanted just
> nulls, I could put nulls in the call, but if I want the actual default,
> I'd have to find the definition and copy-paste from there.

Thank you, that's what I wanted to hear.

>
> --
> Stanislav Malyshev, Software Architect
> SugarCRM: http://www.sugarcrm.com/
> (408)454-6900 ext. 227

--
Regards,
Shein Alexey

Adam Jon Richardson

14 years ago

On Wed, Apr 18, 2012 at 1:56 AM, Alexey Shein <confik@gmail.com> wrote:

> Hi!
>
> My opinion is that solution tries to overcome bad consequences of
> legacy code, when it's not feasible to change something without
> breakage a lot of code, although the real solution is to refactor
> functions with long variable lists (as Uncle Bob says), maybe this
> should be noted in documentation of this feature.
>

I disagree with the suggestion that the "real" solution is to refactor
functions. Is a function with 3 parameters too long?

function foo($val1, $val2 = true, $val3 = false) { /* code */}

If I want to pass in a value to the third argument but keep the default
value for the second argument, I have to look up what the default is and
then paste/type it in as the second argument.

Being able to just call foo($val1 = "something", , $val3 = true) lets me
set a specific value for the 3rd argument without having to do any hunting
down of the 2nd default value or copying/pasting/typing in the argument.

From my perspective, this solution is more about maintaining defaults and
improving developer experience than about problems with legacy code, and it
holds value for any function that has more than one default value.

Adam

Daniel Macedo

14 years ago

I'll keep going offtopic a bit more.
I would rather see named parameters implemented *prior* to this.
Although maybe not instead, I think both might have their place.

On Wed, Apr 18, 2012 at 08:43, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:

>
> Something like
>
> setcookie( name:'mycookie', value:'abc', secure:true, httponly:true );
>
> is really nice to have. Source code will be much more readable.
>

I agree with this! But for short array syntax we kept the => as in
$array = ["foo" => "bar"];
Not sure if this was a limitation, lack of that suggestion or a
decision; but the shortest syntax it's still not... (as Yoda would
say!)

$array = ["foo": "bar"]; doesn't look weird to me, plenty readable,
and that's the shortest!

> This is getting off-topic, but while we're here, I think:
>
> setcookie($name => 'mycookie', $value => 'abc');

Not sure what the optimal PHP syntax should be for named parameters,
but I think this isn't it.


Back to the topic at hand.
There doesn't need to be a lot of parameters on a function for this to
be useful, even with IDE assistance.

For a proper refactoring, where you'd only needing to change the
function definition. In calls where the optional parameters would call
a 'default', they should in fact be the default. Where as now we'd
have to go through all the function calls for any case where we
statically used the default and change it to the new default...

But I couldn't support the comma train, for the insane «lots of
parameters» case, would hate to read some fn($some,,,, $var,,, $other)
call.
I'd rather reuse a reserved word like 'default' (or even get a shorter one?)

Peter Cowburn

14 years ago

On 18 April 2012 10:25, Daniel Macedo <admacedo@gmail.com> wrote:

> But I couldn't support the comma train, for the insane «lots of
> parameters» case, would hate to read some fn($some,,,, $var,,, $other)
> call.
> I'd rather reuse a reserved word like 'default' (or even get a shorter
> one?)
>
>

How about "null"? (Tongue firmly in cheek)

Daniel Macedo

14 years ago

You can't do that, NULL is a perfectly acceptable value to pass into a
function, you wouldn't be able to know when you wanted to pass NULL or
use the default value, e.g.:

function fn1($a = false, $b = nul, $c = 1)
{
var_dump($a, $b. $c);
}

// your idea
fn1(NULL, NULL, 10), // NULL, NULL, 10

// vs. using a reserved word
fn1(default, default, 10), // FALSE, NULL, 10

Cheers,


On Wed, Apr 18, 2012 at 11:21, Peter Cowburn <petercowburn@gmail.com> wrote:

Peter Cowburn

14 years ago

On 18 April 2012 11:38, Daniel Macedo <admacedo@gmail.com> wrote:

> You can't do that, NULL is a perfectly acceptable value to pass into a
> function, you wouldn't be able to know when you wanted to pass NULL or
> use the default value, e.g.:
>
>

You totally missed the point: hinting, not-so-subtly, at the
long-established convention to use null as the default value. I guess my
humour doesn't come across so well on the list.  Lets get back to
discussing ",,," parameters.

Lester Caine

14 years ago

Daniel Macedo wrote:

> function fn1($a = false, $b = nul, $c = 1)
> {
>    var_dump($a, $b. $c);
> }
>
> // your idea
> fn1(NULL, NULL, 10), // NULL, NULL, 10
>
> // vs. using a reserved word
> fn1(default, default, 10), // FALSE, NULL, 10

Of cause one little problem here is if the default gets changed in the 'library'
then every use of 'default' would need to be checked.
Of cause the same problem applies with changing defaults that are used by
leaving out the latter parameters on the list, but this just adds another area
that needs cross checking. Always including the parameters needed is the safe
way in many cases anyway! So leaving them off can be bad practice anyway?

--
Lester Caine - G8HFL
-----------------------------
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk//
Firebird - http://www.firebirdsql.org/index.php

Arvids Godjuks

14 years ago

I personally would vote for the "default" keyword if I want to skip the
param rather than just doing it with , - the keyword approach is much more
readable and error prone.

Stefan Neufeind

14 years ago

On 04/18/2012 01:05 PM, Arvids Godjuks wrote:

> I personally would vote for the "default" keyword if I want to skip the
> param rather than just doing it with , - the keyword approach is much more
> readable and error prone.

I also agree that the "comma-train" (multiple commas without anything
inbetween) might be too confusing and that using a default-keyword
(whatever it might be) might imho be an option.


Regards,
Stefan

Daniel Macedo

14 years ago

I am sorry Lester, but you seem to be getting the "optional parameter"
backwards.

You are supposed to use the optional parameters when it's up to the
library/function definition to determine it's value, not when the
value is what you want for this particular function call!

What you seem to be currently reading from a function definition when
using it is i.e.: "This parameter's default is TRUE? That's what I
need it to be here, skip typing it!"
This is the wrong approach for proper design, and you will have a lot
of headaches when refactoring!

If you use them *properly*, what you read when using the function call
is "I don't need/want to change the default value for this optional
parameter, use what the library determines!", this is the proper way
to use a function call with optional parameters, that will give you a
LOT less headaches!


The problem with the way things work currently, is that the default
value, is now *hard coded* into all function calls were you wanted to
use the default, but needed to define a latter optional parameter!

Hence the proper use would be to either use a reserved word, or having
named parameters.
I personally think there's a place for having both.

Lester Caine

14 years ago

Daniel Macedo wrote:

> I am sorry Lester, but you seem to be getting the "optional parameter"
> backwards.
>
> You are supposed to use the optional parameters when it's up to the
> library/function definition to determine it's value, not when the
> value is what you want for this particular function call!
>
> What you seem to be currently reading from a function definition when
> using it is i.e.: "This parameter's default is TRUE? That's what I
> need it to be here, skip typing it!"
> This is the wrong approach for proper design, and you will have a lot
> of headaches when refactoring!
>
> If you use them*properly*, what you read when using the function call
> is "I don't need/want to change the default value for this optional
> parameter, use what the library determines!", this is the proper way
> to use a function call with optional parameters, that will give you a
> LOT less headaches!

I'm reading things exactly has they are written ...
If I am USING a later optional parameter, then with well written libraries I
also often need to change the earlier optional ones. Since my IDE provides an
overview of how the defaults have been defined I can check, but I *HAVE* been
stung by changes of a default value in the library which then messed up
operation requiring the optional parameter to be set. Adding the ability of also
having to check earlier optional parameters just seems adding to the agro
without any real gain! I'm not talking about MY use of this, but is inclusion
without thought in library code.

> The problem with the way things work currently, is that the default
> value, is now*hard coded*  into all function calls were you wanted to
> use the default, but needed to define a latter optional parameter!
>
> Hence the proper use would be to either use a reserved word, or having
> named parameters.
> I personally think there's a place for having both.

Yet another reason for wanting a LTS version of PHP that we can use without
worrying about all these 'extras'. Named parameters have been discussed in the
past, and not taken forward. If you NEED named parameters, simply use a
parameter hash ... which is the normal practice for 'heavy' parameter passing
anyway.

Similarly if I could trust the library maintainers NOT to change a default, then
'default' might make sense, but to be honest trying to keep libraries working is
as bad nowadays as keeping my own code working as well. I've just finished 3
heavy days making the bulk of the bitweaver code 'strict' safe. Much of the code
still needs extra work, but the bulk of what I USE is now clean. NOW I have to
do the same on the libraries that it uses ... In the absence of a GOOD guide to
'preferred practice' rewriting the bw code to include ALL the public/private
additions will have to wait.

--
Lester Caine - G8HFL
-----------------------------
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk//
Firebird - http://www.firebirdsql.org/index.php

Stas Malyshev

14 years ago

Hi!

> I'll keep going offtopic a bit more.
> I would rather see named parameters implemented *prior* to this.

I'd rather see named parameters implemented already in 5.0. But somehow
that didn't happen :) It's on my todo list though if nobody beats me to it.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Yasuo Ohgaki

14 years ago

Hi,

Just a though for named parameter, since it seems
its becoming a relevant topic now.

2012/4/18 Daniel Macedo <admacedo@gmail.com>:

> I'll keep going offtopic a bit more.
> I would rather see named parameters implemented *prior* to this.
> Although maybe not instead, I think both might have their place.
>
> On Wed, Apr 18, 2012 at 08:43, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
>>
>> Something like
>>
>> setcookie( name:'mycookie', value:'abc', secure:true, httponly:true );
>>
>> is really nice to have. Source code will be much more readable.
>>
>
> I agree with this! But for short array syntax we kept the => as in
> $array = ["foo" => "bar"];
> Not sure if this was a limitation, lack of that suggestion or a
> decision; but the shortest syntax it's still not... (as Yoda would
> say!)
>
> $array = ["foo": "bar"]; doesn't look weird to me, plenty readable,
> and that's the shortest!

Object can be used as named parameter in JavaScript.

// Define function to take one "argument", which is in fact an object:
function fnParseInt( oArg ){
return parseInt( oArg.number, oArg.radix );
}
// Which you then call like this (pass in an object literal):
fnParseInt( { number : 'afy', radix : 36 } );

If there is a JSON like syntax for PHP objects, named parameter
can be implemented just like this. It would not work for function
that accepts object as first parameter, but who cares?

Pros.
- simple
- everyone are used to JSON and JavaScript now a days
- supporting JSON syntax is convenient for other purpose
- internal functions may support this.
(Single object parameter for this)
- coexists func($a,,,,$b)

Cons
- loose type hints
- loose default values for params
- yet another script injection risk if we do "eval($json_data)"
(json_decode() should be used, anyway.)

We may use array to emulate JavaScript, though.
fnParseInt(['number'=>'afy', 'radix'=>36]);

It's many typing compare to other languages, but it works.
If we are going to have array like syntax, we may  just
use array.

Anyway, I'm not sure what happened for annotation.
Annotation maybe used as type hints, etc.

Just an idea that I think of now.
There might be many better ideas for named parameter.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Matthew Weier O'Phinney

14 years ago

On 2012-04-19, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:

> Just a though for named parameter, since it seems
> its becoming a relevant topic now.
>
> 2012/4/18 Daniel Macedo <admacedo@gmail.com> :

<snip>

> > I agree with this! But for short array syntax we kept the =>  as in
> > $array = ["foo" => "bar"];
> > Not sure if this was a limitation, lack of that suggestion or a
> > decision; but the shortest syntax it's still not... (as Yoda would
> > say!)
> >
> > $array = ["foo": "bar"]; doesn't look weird to me, plenty readable,
> > and that's the shortest!
>
> Object can be used as named parameter in JavaScript.
>
> // Define function to take one "argument", which is in fact an object:
> function fnParseInt( oArg ){
>    return parseInt( oArg.number, oArg.radix );
> }
> // Which you then call like this (pass in an object literal):
> fnParseInt( { number : 'afy', radix : 36 } );
>
> If there is a JSON like syntax for PHP objects, named parameter
> can be implemented just like this. It would not work for function
> that accepts object as first parameter, but who cares?

This is the workaround many of us are already using, though typically
passing an array, not an object. With 5.4, the notation is almost
usable, as you can simply do the square brackets:

fnParseInt([ "number" => "afy", "radix" => 36 ]);

> Pros.
>   - simple
>   - everyone are used to JSON and JavaScript now a days
>   - supporting JSON syntax is convenient for other purpose
>   - internal functions may support this.
>      (Single object parameter for this)
>   - coexists func($a,,,,$b)
>
> Cons
>   - loose type hints
>   - loose default values for params

While the solution is indeed simple, the cons loom very large -- you
end up needing to perform a lot of logic internally in the function in
order to support default values as well as enforce types. That kind of
boilerplate gets very old after a while.

Named parameters would be ideal, but being able to skip parameters will
definitely work in the interim.

--
Matthew Weier O'Phinney
Project Lead            | matthew@zend.com
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

Lester Caine

14 years ago

Adam Jon Richardson wrote:

> I disagree with the suggestion that the "real" solution is to refactor
> functions. Is a function with 3 parameters too long?
>
> function foo($val1, $val2 = true, $val3 = false) { /* code */}
>
> If I want to pass in a value to the third argument but keep the default
> value for the second argument, I have to look up what the default is and
> then paste/type it in as the second argument.

OK we are back in the debate on what needs to be IN the runtime handling ...
Eclipse gives be a pop-up with the parameter set so I know what is expected. I
don't even need to go and 'lookup' anything.

Where the parameter chain is more than a couple of entries, then use a parameter
hash. Always a lot easier anyway when handling SQL type activities as one can
add stuff later and the order is irrelevant anyway.

When one uses the right support tools is this really something to be wasting
even more time on?

--
Lester Caine - G8HFL
-----------------------------
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk//
Firebird - http://www.firebirdsql.org/index.php

Adam Harvey

14 years ago

On 18 April 2012 06:54, Stas Malyshev <smalyshev@sugarcrm.com> wrote:

> create_query("deleted=0", "name",,, /*report_errors*/ true);

I like it. At the risk of bikeshedding, can we have a token rather
than just a series of commas — say reusing the default keyword so it
would look like this instead:

create_query("deleted=0", "name", default, default, /*report_errors*/ true);

That seems easier to read to my eyes than counting commas.

Thanks,

Adam

Ferenc Kovacs

14 years ago

On Wed, Apr 18, 2012 at 12:54 AM, Stas Malyshev <smalyshev@sugarcrm.com>wrote:

> Hi!
>
> One of the annoying things I've encountered in working with PHP was
> dealing with functions having long optional parameter lists, especially
> if you need to change only the last one - you have to copy all the
> defaults. Full named params implementation would solve it, probably, but
> before we have that here's an easier solution for part of the problem:
>
> https://wiki.php.net/rfc/skipparams
>
> Basically, it allows you to do this:
>
> create_query("deleted=0", "name",,, /*report_errors*/ true);
>
> Implementation of it is here:
> https://github.com/smalyshev/php-src/tree/skip_params
> All things described in RFC seem to be working, please tell me if I
> forgot anything. If it's accepted I'll also add tests, etc. of course.
>
>

I would prefer Named parameters, but this is still better than the current
situation.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

Yasuo Ohgaki

14 years ago

Hi,

2012/4/18 Ferenc Kovacs <tyra3l@gmail.com>:

> I would prefer Named parameters, but this is still better than the current
> situation.

Something like

setcookie( name:'mycookie', value:'abc', secure:true, httponly:true );

is really nice to have. Source code will be much more readable.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Ryan McCue

14 years ago

Yasuo Ohgaki wrote:

> Something like
>
> setcookie( name:'mycookie', value:'abc', secure:true, httponly:true );
>
> is really nice to have. Source code will be much more readable.

This is getting off-topic, but while we're here, I think:

setcookie($name => 'mycookie', $value => 'abc');

...would be better.

--
Ryan McCue
<http://ryanmccue.info/>

Yasuo Ohgaki

14 years ago

Hi,

2012/4/18 Ryan McCue <lists@rotorised.com>:

> Yasuo Ohgaki wrote:
>>
>> Something like
>>
>> setcookie( name:'mycookie', value:'abc', secure:true, httponly:true );
>>
>> is really nice to have. Source code will be much more readable.
>
>
> This is getting off-topic, but while we're here, I think:
>
> setcookie($name => 'mycookie', $value => 'abc');
>
> ...would be better.

Real life code would be something like

setcookie($name => $my_cookie_name, $value => $value);

It's confusing.
Off topic anyway.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Christian Kaps

14 years ago

Am 18.04.2012 09:18, schrieb Ferenc Kovacs:

> On Wed, Apr 18, 2012 at 12:54 AM, Stas Malyshev
> <smalyshev@sugarcrm.com>wrote:
>
>> Hi!
>>
>> One of the annoying things I've encountered in working with PHP was
>> dealing with functions having long optional parameter lists,
>> especially
>> if you need to change only the last one - you have to copy all the
>> defaults. Full named params implementation would solve it, probably,
>> but
>> before we have that here's an easier solution for part of the
>> problem:
>>
>> https://wiki.php.net/rfc/skipparams
>>
>> Basically, it allows you to do this:
>>
>> create_query("deleted=0", "name",,, /*report_errors*/ true);
>>
>> Implementation of it is here:
>> https://github.com/smalyshev/php-src/tree/skip_params
>> All things described in RFC seem to be working, please tell me if I
>> forgot anything. If it's accepted I'll also add tests, etc. of
>> course.
>>
>>
> I would prefer Named parameters, but this is still better than the
> current
> situation.

Every time a new language construct is proposed, the first arguments
against are:
- This is confusing syntax
- It is hard to read
- And so one

Stas, and I think you are one of the core developers which raises his
voice as first, against new language constructs. And now you propose
such a construct. But the best is you show the optimal solution(named
parameters) for this problem in your post, and suggests this proposal as
work around for it. So if this proposal gets implemented and maybe in
the future named parameters gets also implemented. Have we then two
solutions for this problem. Or does we revert your current proposal.

Sorry, its not against you but for me it seems more as a quick shot.

Just my 2 cents
Christian

高春辉

14 years ago

+1.

I LIKE https://wiki.php.net/rfc/namedparameters , please change OLD decide.

-----邮件原件-----
发件人: Ferenc Kovacs [mailto:tyra3l@gmail.com]
发送时间: 2012年4月18日 15:19
收件人: Stas Malyshev
抄送: PHP Internals
主题: Re: [PHP-DEV] [RFC] skipping optional parameters

On Wed, Apr 18, 2012 at 12:54 AM, Stas Malyshev <smalyshev@sugarcrm.com>wrote:

> Hi!
>
> One of the annoying things I've encountered in working with PHP was
> dealing with functions having long optional parameter lists,
> especially if you need to change only the last one - you have to copy
> all the defaults. Full named params implementation would solve it,
> probably, but before we have that here's an easier solution for part of the problem:
>
> https://wiki.php.net/rfc/skipparams
>
> Basically, it allows you to do this:
>
> create_query("deleted=0", "name",,, /*report_errors*/ true);
>
> Implementation of it is here:
> https://github.com/smalyshev/php-src/tree/skip_params
> All things described in RFC seem to be working, please tell me if I
> forgot anything. If it's accepted I'll also add tests, etc. of course.
>
>

I would prefer Named parameters, but this is still better than the current situation.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

Pierre Joye

14 years ago

hi Stas,

On Wed, Apr 18, 2012 at 12:54 AM, Stas Malyshev <smalyshev@sugarcrm.com> wrote:

> create_query("deleted=0", "name",,, /*report_errors*/ true);

I like the concept, a lot :)

Indeed there is the "code written for only this version and above",
from a syntax point of view. But that's something I do not care much
about.

Cheers,

--
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

Matthew Weier O'Phinney

14 years ago

On 2012-04-17, Stas Malyshev <smalyshev@sugarcrm.com> wrote:

> One of the annoying things I've encountered in working with PHP was
> dealing with functions having long optional parameter lists, especially
> if you need to change only the last one - you have to copy all the
> defaults. Full named params implementation would solve it, probably, but
> before we have that here's an easier solution for part of the problem:
>
> https://wiki.php.net/rfc/skipparams
>
> Basically, it allows you to do this:
>
> create_query("deleted=0", "name",,, /*report_errors*/ true);

I actually had a need for either this or named parameters this past
week, and as such would love to see one or the other in place. While I'd
personally prefer named arguments, this approach would work in most
places I've needed them.

My one comment, which others have raised, is readability of multiple
commas -- TBH, at first glance it has the appearance of a mistake. I
think those suggesting a keyword such as "default" make a good point in
this regard -- it makes it 100% clear that you want the default value
for the argument in that position. This also presents an improvement
over current usage, as you're not hard-coding values in your function
calls themselves -- particularly as the defaults could change in future
refactors.

--
Matthew Weier O'Phinney
Project Lead            | matthew@zend.com
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

Stas Malyshev

14 years ago

Hi!

> My one comment, which others have raised, is readability of multiple
> commas -- TBH, at first glance it has the appearance of a mistake. I
> think those suggesting a keyword such as "default" make a good point in
> this regard -- it makes it 100% clear that you want the default value

I wouldn't mind default but that means another keyword which means
another disruption (default would be pretty common method name, etc.)

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Nikita Popov

14 years ago

On Wed, Apr 18, 2012 at 10:24 PM, Stas Malyshev <smalyshev@sugarcrm.com> wrote:

> Hi!
>
>> My one comment, which others have raised, is readability of multiple
>> commas -- TBH, at first glance it has the appearance of a mistake. I
>> think those suggesting a keyword such as "default" make a good point in
>> this regard -- it makes it 100% clear that you want the default value
>
> I wouldn't mind default but that means another keyword which means
> another disruption (default would be pretty common method name, etc.)

"default" is already a reserved keyword. It's used in switch
constructs. So it is safe to use :)

Nikita

Stas Malyshev

14 years ago

Hi!

> "default" is already a reserved keyword. It's used in switch
> constructs. So it is safe to use :)

Ah, silly me, indeed it is. Then I guess it doesn't hurt to add it as an
option. Will do.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Florian Anderiasch

14 years ago

On 04/18/2012 11:04 PM, Stas Malyshev wrote:

> Hi!
>
>> "default" is already a reserved keyword. It's used in switch
>> constructs. So it is safe to use :)
>
> Ah, silly me, indeed it is. Then I guess it doesn't hurt to add it as an
> option. Will do.

I can't estimate the amount of breakage, but what about using underscore
(literal _) without quotation marks?

In functional languages it's sometimes the "default match" in pattern
matching, that's at least a loose conversion and I hope not many people
would do
define("_", <whatever>) in their code.

The question now is, if create_query("a", _, _, _, true) really looks
better...

Greetings,
Florian

Stas Malyshev

14 years ago

Hi!

> I can't estimate the amount of breakage, but what about using underscore
> (literal _) without quotation marks?

This one is taken. See: http://us2.php.net/_

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Ferenc Kovacs

13 years ago

2012.04.20. 8:08, "Stas Malyshev" <smalyshev@sugarcrm.com> ezt írta:

>
> Hi!
>
> > I can't estimate the amount of breakage, but what about using underscore
> > (literal _) without quotation marks?
>
> This one is taken. See: http://us2.php.net/_
>
> --
> Stanislav Malyshev, Software Architect
> SugarCRM: http://www.sugarcrm.com/
> (408)454-6900 ext. 227
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>

Hi,

what is the status of the rfc?
were there any reasons why you didn't called for votes?
Personally I would prefer named parameters also, and I think that we are
too close to the 5.5 feature freeze, but somebody asked why did the
progress stopped and I don't think that there were any showstopper issues.

Stas Malyshev

13 years ago

Hi!

> what is the status of the rfc?
> were there any reasons why you didn't called for votes?
> Personally I would prefer named parameters also, and I think that we are
> too close to the 5.5 feature freeze, but somebody asked why did the
> progress stopped and I don't think that there were any showstopper issues.

There are some issues with making internal functions compatible with
this and fixing all the tests, and I didn't have enough time to get
through it yet. I hope to get back to it in the near future, but
probably not in time for 5.5 if its schedule stays as it is now. The
concept is basically done but there are all kinds of cleanups to be done
and tests to be fixed and so on and it takes time, unfortunately.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Patrick ALLAERT

14 years ago

2012/4/18 Matthew Weier O'Phinney <weierophinney@php.net>:

> My one comment, which others have raised, is readability of multiple
> commas -- TBH, at first glance it has the appearance of a mistake. I
> think those suggesting a keyword such as "default" make a good point in
> this regard -- it makes it 100% clear that you want the default value
> for the argument in that position. This also presents an improvement
> over current usage, as you're not hard-coding values in your function
> calls themselves -- particularly as the defaults could change in future
> refactors.

I think we should only support optional parameters using the "default"
keyword as it would probably make things more consistent and less
error prone:

function foo( $bar = "bar", $foo = "foo", $baz = "baz" ) { ... }

foo(,,, "myBaz"); // Thinking changing the value of $baz, but there's
one too much "," and PHP won't complain because of too many arguments
used!

Additionally, we might also want to think about
call_user_func()/call_user_func_array().

If for the former it could work with:

call_user_func( "foo", , , "myBaz" );

What about the latter?

call_user_func_array( "foo", [2 => "myBaz"] ); // ? Evaluating the
element at index 0 would cause a notice, but would result in a NULL,
so I would say that NULL is to be used as first parameter.

I have no clue on how to correctly support skipped default values in
that case since there is a big difference in supporting the "default"
keyword inside the method call vs. as an element of an array.
Ideas are welcome!

Last but not least: if skipping optional parameters is implemented,
should we consider the support of default values on the leftmost side
of functions?

function foo( $bar = "bar", $baz ) { ... }

It could make sense that $bar followed by $baz is semantically better
than the opposite for technical reason, and this could be called
using:

foo( default, "baz")
or:
foo(, "baz")

Matthew Weier O'Phinney

14 years ago

On 2012-04-19, Patrick ALLAERT <patrickallaert@php.net> wrote:

> 2012/4/18 Matthew Weier O'Phinney <weierophinney@php.net> :
> > My one comment, which others have raised, is readability of multiple
> > commas -- TBH, at first glance it has the appearance of a mistake. I
> > think those suggesting a keyword such as "default" make a good point in
> > this regard -- it makes it 100% clear that you want the default value
> > for the argument in that position. This also presents an improvement
> > over current usage, as you're not hard-coding values in your function
> > calls themselves -- particularly as the defaults could change in future
> > refactors.
>
> I think we should only support optional parameters using the "default"
> keyword as it would probably make things more consistent and less
> error prone:
>
> function foo( $bar = "bar", $foo = "foo", $baz = "baz" ) { ... }
>
> foo(,,, "myBaz"); // Thinking changing the value of $baz, but there's
> one too much "," and PHP won't complain because of too many arguments
> used!
>
> Additionally, we might also want to think about
> call_user_func()/call_user_func_array().
>
> If for the former it could work with:
>
> call_user_func( "foo", , , "myBaz" );
>
> What about the latter?
>
> call_user_func_array( "foo", [2 => "myBaz"] ); // ? Evaluating the
> element at index 0 would cause a notice, but would result in a NULL,
> so I would say that NULL is to be used as first parameter.

I actually would argue you shouldn't skip parameters when using
call_user_func(); even with call_user_func_array(), I'd argue that named
parameters is the only way I'd want to allow skipping parameters.

The reason I argue this is because when dynamically calling a function,
you typically don't actually know exactly what the function/method is --
and thus making any assumptions about the signature other than required
parameters is simply begging for problems.

<snip>

> Last but not least: if skipping optional parameters is implemented,
> should we consider the support of default values on the leftmost side
> of functions?
>
> function foo( $bar = "bar", $baz ) { ... }
>
> It could make sense that $bar followed by $baz is semantically better
> than the opposite for technical reason, and this could be called
> using:
>
> foo( default, "baz")
> or:
> foo(, "baz")

I'd argue no -- as required arguments should always have precedence over
optional ones in the signature when it comes to position.

--
Matthew Weier O'Phinney
Project Lead            | matthew@zend.com
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

Pierrick Charron

14 years ago

I like the idea. In fact I started to implement it some time ago, but your
patch is really cleaner than mine :)
Otherwise I would prefer to insert the default keyword to make the code
more readable. It might be hard to count when you have something like

function(,,,,true);

P.

On 17 April 2012 18:54, Stas Malyshev <smalyshev@sugarcrm.com> wrote:

Gustavo Lopes

14 years ago

On Wed, 18 Apr 2012 00:54:41 +0200, Stas Malyshev <smalyshev@sugarcrm.com>
wrote:

> [snip]
> https://wiki.php.net/rfc/skipparams
>
> [snip]

«For internal functions, parameter parser will ignore the NULLs, thus
leaving the defaults supplied by the caller intact. Again, skipping
non-optional parameter is an error.»

This would cause a lot of problems. Basically, all the functions that rely
on ZEND_NUM_ARGS() will have to be changed. You can't tell if a parameter
was passed or not by relying on it.

For instance, if you have ZEND_NUM_ARGS() yielding x, but skipped arg x -
y, now the variable for the argument x - y won't be touched but the
function will think the argument was supplied. Segfaults and unpredictable
behavior due to uninitialized values and other logic errors will follow.

Of course, this because internal functions don't have "default values", at
least not the same way user-land functions do.

This seems a lot of trouble for a feature that, in my opinion, even if it
could be implemented without problems, won't be useful enough to overcome
the strong presumption that exists against new syntax changes. Maybe I'm
lucky, but the problem you're trying to solve is at most an occasional
minor nuisance for me. Plus, this solution encourages bad behavior. If we
want to properly support long argument lists (and I'm not we do), we have
to have named parameters.

--
Gustavo Lopes

Stas Malyshev

14 years ago

Hi!

> This would cause a lot of problems. Basically, all the functions that rely
> on ZEND_NUM_ARGS() will have to be changed. You can't tell if a parameter
> was passed or not by relying on it.

ZEND_NUM_ARGS() would probably work since IIRC it relies on stack size,
not on varargs return. Yes, that means ZEND_NUM_ARGS() and varargs would
return different things, and that means when you manually fetch args
from stack you'd have to check for NULLs - but these cases should be
quite rare and can be handled on case-by-case basis. Most functions
don't - and shouldn't - use manual stack parsing, and for those
everything would work just fine.

> the strong presumption that exists against new syntax changes. Maybe I'm
> lucky, but the problem you're trying to solve is at most an occasional
> minor nuisance for me. Plus, this solution encourages bad behavior. If we

Please look at the RFC, it has links to multiple requests from users for
this feature.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Gustavo Lopes

14 years ago

On Wed, 18 Apr 2012 22:34:23 +0200, Stas Malyshev <smalyshev@sugarcrm.com>
wrote:

> Hi!
>
>> This would cause a lot of problems. Basically, all the functions that
>> rely
>> on ZEND_NUM_ARGS() will have to be changed. You can't tell if a
>> parameter
>> was passed or not by relying on it.
>
> ZEND_NUM_ARGS() would probably work since IIRC it relies on stack size,
> not on varargs return.

Not sure what you mean here. What is this "varargs return"? As far as I
see, a NULL pointer is pushed to the stack and ZEND_NUM_ARGS() will return
the size of the stack; in this case, the index of the last argument, not
the number of arguments actually passed. But even if it returned the
number of arguments passed, it would be irrelevant because you would not
know *which* arguments were passed.

> Yes, that means ZEND_NUM_ARGS() and varargs would
> return different things, and that means when you manually fetch args
> from stack you'd have to check for NULLs - but these cases should be
> quite rare and can be handled on case-by-case basis. Most functions
> don't - and shouldn't - use manual stack parsing, and for those
> everything would work just fine.

This is a straw man argument as I was not addressing manual stack parsing,
but, in any case, I think it reveals even a stronger argument against this
addition. There is no reason to think functions shouldn't do manual
argument parsing. In fact, they do, and there are even many ways to have
them do it -- zpp with Z*/z*/Z+/z+, _zend_get_parameters_array,
zend_get_parameters_ex and more. These are not one or two and they will
stop working,

But to my argument: many functions use ZEND_NUM_ARGS() to determine if an
argument was passed. Sometimes, this is the only way to determine if an
argument was passed -- for instance, if we're using zpp with "l", see
mt_srand() for an example. For a more drastic example, see the
implementation of PDOStatement::fetchAll().

Now, you may argue that this is a bad practice because you can't pass an
argument with a value with same effect of not passing it, and I would
agree. In fact, I've just gone through a great deal of trouble to avoid
having internal functions with this kind of behavior. See
https://github.com/cataphract/php-src/blob/intl_calendar/ext/intl/calendar/calendar_methods.cpp#L399
-- and guess what -- your feature would break this.

Good or bad, usage of ZEND_NUM_ARGS() is all over the place:

glopes@nebm:~/php/php-src$ git grep -n ZEND_NUM_ARGS | grep -v zend_parse
| grep -v zend_get_parameters | wc -l
364

(I removed its usage in zpp and similar)

>
>> the strong presumption that exists against new syntax changes. Maybe I'm
>> lucky, but the problem you're trying to solve is at most an occasional
>> minor nuisance for me. Plus, this solution encourages bad behavior. If
>> we
>
> Please look at the RFC, it has links to multiple requests from users for
> this feature.

I don't think this is a strong argument, I could find a number of
proponents for every crazy feature.

--
Gustavo Lopes

Stas Malyshev

14 years ago

Hi!

> Not sure what you mean here. What is this "varargs return"? As far as I

zend_parse_parameters has varargs options. See how var_dump is implemented.

> the number of arguments actually passed. But even if it returned the
> number of arguments passed, it would be irrelevant because you would not
> know *which* arguments were passed.

I would know. Unless you're using "+" in zend_parse_parameters but
actually have positional arguments - so far I know no such functions but
if I discover any I'll see how to handle it.

> addition. There is no reason to think functions shouldn't do manual
> argument parsing. In fact, they do, and there are even many ways to have

There is. We have API for argument passing, functions should use it. If
you're not using APIs, you'd have to fix your function when something
changes. This is why we have APIs.

> them do it -- zpp with Z*/z*/Z+/z+, _zend_get_parameters_array,
> zend_get_parameters_ex and more. These are not one or two and they will
> stop working,

Z params won't stop working, why should they? They'd just have nulls
there. You'd have to handle these nulls of course.

> But to my argument: many functions use ZEND_NUM_ARGS() to determine if an
> argument was passed. Sometimes, this is the only way to determine if an
> argument was passed -- for instance, if we're using zpp with "l", see
> mt_srand() for an example. For a more drastic example, see the
> implementation of PDOStatement::fetchAll().

Passing default to mt_srand makes little sense since mt_srand doesn't
have default documented. But we can define it as being 0. Of course,
mt_srand(default) will be different from mt_srand() then, but so what?
It doesn't break anything.
For fetchAll() NULL there might be indeed unexpected - so we may think
about either fixing such cases or letting zend_parse_parameters know if
this parameter can handle default (most can) or it does some fancy stuff
with argument dependencies.

> Now, you may argue that this is a bad practice because you can't pass an
> argument with a value with same effect of not passing it, and I would
> agree. In fact, I've just gone through a great deal of trouble to avoid
> having internal functions with this kind of behavior. See
> https://github.com/cataphract/php-src/blob/intl_calendar/ext/intl/calendar/calendar_methods.cpp#L399
> -- and guess what -- your feature would break this.

I don't think it will break that. You may have to do a bit more work to
do it, but it can be done. Though frankly I have no idea what that code
is supposed to do and suspect it is not really a good API if it requires
this level of variation in one function. It basically doing all
parameters handling by itself instead of using system API and this is
usually a bad sign. With or without this feature, I would seriously
consider redesigning it and making it simpler. But again, I have no clue
what this function is supposed to do. Which, BTW, is pretty bad since
apparently it is committed to main code base without having any
comments, prototypes, RFCs, documentation or anything - how one would be
supposed to make any use of it?

> Good or bad, usage of ZEND_NUM_ARGS() is all over the place:
>
> glopes@nebm:~/php/php-src$ git grep -n ZEND_NUM_ARGS | grep -v zend_parse
> | grep -v zend_get_parameters | wc -l
> 364

Yeah, I've fallen into this trap too initially. Most of those are
multiline calls to zend_parse_parameters and checks that are completely
harmless. There will be some cases where they aren't - like when default
parameters depend on each other like in fetchAll() - but those can be
handled. Functions like yours above are extremely rare and should be.
And if we ever get names params you'd have to rewrite it anyway so I'd
seriously consider starting to think how it would work with it then. I'd
go with "+" or "*" but that's just a guess.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Gustavo Lopes

14 years ago

On Wed, 18 Apr 2012 23:35:37 +0200, Stas Malyshev <smalyshev@sugarcrm.com>
wrote:

Before addressing the points individually, I have to reiterate that
internal functions have no concept of default values. I think this
proposal or a named arguments proposal is *really* ill-advised without
introducing proper default values for internal functions. Then the engine
could pass those default values on the stack and we would have none of
these problems.

Instead, we have lots of functions that will not only break (see below for
a definition of "break"), but break subtly (e.g. uninitialized values) and
only under very particular conditions (using this new feature). Plus, they
increase the complexity of doing argument parsing (surely no one will
forget checking for NULLs). All the functions that use ZEND_NUM_ARGS()
have to be reviewed and the PHP core is just the tip of the iceberg.

>> the number of arguments actually passed. But even if it returned the
>> number of arguments passed, it would be irrelevant because you would not
>> know *which* arguments were passed.
>
> I would know. Unless you're using "+" in zend_parse_parameters but
> actually have positional arguments - so far I know no such functions but
> if I discover any I'll see how to handle it.

I was talking about ZEND_NUM_ARGS(). My point is that ZEND_NUM_ARGS() now
gives near-0 useful information. Yes, you can know which arguments were
passed through other means, but not via ZEND_NUM_ARGS(), which what many
functions are currently using.

> Z params won't stop working, why should they? They'd just have nulls
> there. You'd have to handle these nulls of course.

When I say this "breaks" current functions, I don't mean they will never
work again. I mean that, due to different semantics in argument passing,
they will have to be changed. I'm assuming even the though cases like
PDOStatement::fetchAll() can be fixed.

>
> Passing default to mt_srand makes little sense since mt_srand doesn't
> have default documented. But we can define it as being 0. Of course,
> mt_srand(default) will be different from mt_srand() then, but so what?
> It doesn't break anything.

mt_srand() was a bad example. But let's supposed it didn't initialize
seed. It would be perfectly correct under current semantics, but as far as
I understand under the patch mt_srand(default) would result in
uninitialized data being used.

>> https://github.com/cataphract/php-src/blob/intl_calendar/ext/intl/calendar/calendar_methods.cpp#L399
>> -- and guess what -- your feature would break this.
>
> I don't think it will break that. You may have to do a bit more work to
> do it, but it can be done. Though frankly I have no idea what that code
> is supposed to do and suspect it is not really a good API if it requires
> this level of variation in one function. It basically doing all
> parameters handling by itself instead of using system API and this is
> usually a bad sign.

All it does is check for (PHP) NULLs before using zpp with "l", because
"l" would convert the NULL to 0 and I want to distinguish the two cases,
so that I can use NULL to the same effect of not passing the argument.
This was actually discussed in a pull request a few weeks ago.

>
>> Good or bad, usage of ZEND_NUM_ARGS() is all over the place:
>>
>> glopes@nebm:~/php/php-src$ git grep -n ZEND_NUM_ARGS | grep -v
>> zend_parse
>> | grep -v zend_get_parameters | wc -l
>> 364
>
> Yeah, I've fallen into this trap too initially. Most of those are
> multiline calls to zend_parse_parameters and checks that are completely
> harmless.

Quickly skimming the output with shows that in some cases it's multiline
calls, but those are in a clear minority (most are in if/switch
statements). And remember, this is just PHP core, a small fraction of all
extensions.

--
Gustavo Lopes

Stas Malyshev

14 years ago

Hi!

> Before addressing the points individually, I have to reiterate that
> internal functions have no concept of default values. I think this

I disagree. Most of them certainly do, for example, look at strpos
documentation:

int strpos ( string $haystack , mixed $needle [, int $offset = 0 ] )

$offset is documented as having default of 0. Right now it is
implemented in ad-hoc manner - i.e. you set up the value, but you don't
actually declare it anywhere as some special "default value". But it is
certainly there and for the user it is certainly a default as any other.
We didn't do the best job documenting all these, unfortunately (see
below) but in almost all cases they are there.

> Instead, we have lots of functions that will not only break (see below for
> a definition of "break"), but break subtly (e.g. uninitialized values) and

Any function that passes uninitialized value to optional argument has a
bug and needs to be fixed.

> only under very particular conditions (using this new feature). Plus, they
> increase the complexity of doing argument parsing (surely no one will
> forget checking for NULLs). All the functions that use ZEND_NUM_ARGS()
> have to be reviewed and the PHP core is just the tip of the iceberg.

For 99.9% of the functions there's no increase of complexity since they
already do the right thing. As for the matter of checking for NULL being
too complex - this is the absolutely basic thing you always do in C,
check your pointers, if you write engine code it can't be too complex
for you.

> I was talking about ZEND_NUM_ARGS(). My point is that ZEND_NUM_ARGS() now
> gives near-0 useful information. Yes, you can know which arguments were
> passed through other means, but not via ZEND_NUM_ARGS(), which what many
> functions are currently using.

This is not true. It gives the same information as before, and it is
very useful. The only case where you'd have to take some extra case is
where you do not use recommended APIs but try to fetch arguments from
stack manually or when you have complex dependencies between parameters.
Then you'd have to check for NULLs and decide if you are ready to accept
parameters after that. Not rocket science by any measure.

> When I say this "breaks" current functions, I don't mean they will never
> work again. I mean that, due to different semantics in argument passing,
> they will have to be changed. I'm assuming even the though cases like
> PDOStatement::fetchAll() can be fixed.

Yes, some functions with extremely complex corner cases of parameter
parsing will have to be slightly changed. I'll think how to make this
change easier. Of course, for this functions notion of default
parameters won't work - we may even introduce some API to tell
zend_parse_parameters about it. But for many other functions it may work
just fine. For example, in the same PDO you have:

bool PDOStatement::bindColumn ( mixed $column , mixed &$param [, int
$type [, int $maxlen [, mixed $driverdata ]]] )

Now, what you do if you want to supply $driverdata but don't care about
max length? We have two problems here:
1. Defaults are not documented, so you have zero idea how to get default
behavior without looking into C code.
2. Defaults could change and you'd have to change your code.

Skipping parameters would work here just fine and will use the same
defaults the code uses, with zero pain to the user.

> mt_srand() was a bad example. But let's supposed it didn't initialize
> seed. It would be perfectly correct under current semantics, but as far as
> I understand under the patch mt_srand(default) would result in
> uninitialized data being used.

If it didn't initialize seed, it would be a bug which even now needs to
be fixed. You're not supposed to pass uninitialized values for default
args into zend_parse_parameters, if you code does that, please fix it ASAP.

> All it does is check for (PHP) NULLs before using zpp with "l", because
> "l" would convert the NULL to 0 and I want to distinguish the two cases,
> so that I can use NULL to the same effect of not passing the argument.
> This was actually discussed in a pull request a few weeks ago.

This seems to be a very unusual special case then, which has very little
to do with topic at hand. Probably unique case in our code. Maybe we
need to change zend_parse_parameters API or your function API or do some
other good solution for it, but it's offtopic for now. Feel free to open
a new thread on that, but for purposes of this one the conclusion is -
yes, if you ignore system APIs then there's some work to be done there.
I'll scan the code for such functions and see how to fix it best, thanks
for bringing this issue to my attention. I do not see however that it
somehow invalidates the concept, as 99% of the functions do not do that
and you're not supposed to do that except in very special circumstances.

> Quickly skimming the output with shows that in some cases it's multiline
> calls, but those are in a clear minority (most are in if/switch
> statements). And remember, this is just PHP core, a small fraction of all
> extensions.

Again, unless you ignore zend_parse_parameters API and cook your own
parsing, your extension is just fine, which will cover about 99% of
them. For the rest, I'll look for a solution.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227