RFC: constructor argument promotion

12 years ago by Stas Malyshev — view source — reply

unread

Hi!

https://wiki.php.net/rfc/constructor-promotion

From the first glance, it doesn't seem clear how this syntax would
interact with magic methods - i.e., if you have __get, would access to
$make call it? If not, it's rather un-intuitive since the property is
not defined in the class but magic method is not called.
What would happen with this code:

class Base { public __construct(public $f) {} }
class PublicBase extends Base {
public __construct($param) { __parent::__construct($param); }
}

class Child extends PublicBase { public __construct(public $f) {
$this->f = 42; parent::__construct($f); } }

Note here that whoever writes class Child may not be aware that class
Base even exists and how it is implemented, since he implements against
PublicBase.

What happens if I need to have some arguments that are not properties?
How would one extract phpdoc descriptions for such properties?
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

12 years ago by Lars Strojny — view source — reply

unread

Hi Sean,

thanks for the RFC. Two other questions additionally to what Stas askes:

What about class type hints and array type hints?
If type hints are possible, doesn’t it look too much as real property type hinting?

cu,
Lars

Am 07.08.2013 um 21:47 schrieb Sean Cannella seanc@fb.com:

Everyone -

Hi! Since this is my first post to this list, I'll introduce myself:
I'm an engineer who has been working on HipHop VM in New York for the last half year or so after a long time working at Microsoft on business software and services in multiple hemispheres.

I wanted to get the PHP community's opinion on this feature. See the draft RFC and referenced implementation below:

https://wiki.php.net/rfc/constructor-promotion

What do you all think?

Thanks,
Sean Cannella | Facebook Eng
If you are smart and persistent, you may find the truth. It is not always a reward.

12 years ago by Hannes Magnusson — view source — reply

unread

Everyone -

Hi! Since this is my first post to this list, I'll introduce myself:
I'm an engineer who has been working on HipHop VM in New York for the last half year or so after a long time working at Microsoft on business software and services in multiple hemispheres.

I wanted to get the PHP community's opinion on this feature. See the draft RFC and referenced implementation below:

https://wiki.php.net/rfc/constructor-promotion

Using private and protected there seems really weird.
From globals scope, I am allowed to set private and protected
properties - but just once?

I would assume __construct(protected $bar) would only allow a child
class to set that property.. which implies the ctor itself is
protected.

Also, this seems like a very bad idea. You are encouraging "don't
bother validating your data" ideas.

-Hannes

12 years ago by Leigh — view source — reply

unread

https://wiki.php.net/rfc/constructor-promotion

What do you all think?

I'm not sure what problem this is really trying to solve, the boilerplate
code you mention is very explicit and it is very clear to the reader what
is being done. Each property documented with its type, purpose and
visibility in a common place for easy reference (at the top of the class).
Each property that takes a value from the constructor assigned in the
constructor. Very clear.

To me this seems to be adding unnecessary magic and extra rules to remember
(the contrived case you mention at the bottom of the RFC for example would
actually be incredibly common) at the expense of readability and code
clarity.

12 years ago by Jordi Boggiano — view source — reply

unread

I'm not sure what problem this is really trying to solve, the boilerplate
code you mention is very explicit and it is very clear to the reader what
is being done. Each property documented with its type, purpose and
visibility in a common place for easy reference (at the top of the class).
Each property that takes a value from the constructor assigned in the
constructor. Very clear.

I for one am pretty tired of writing this boilerplate in every second
class I write. Using dependency injection you end up having to write a
LOT of those usually, and constructors typically only contain assignments.

Adding a property means: declaring the property, adding the ctor arg,
adding the assignment in the ctor. You have to write the property name 4
times. With this RFC in it'd come down to writing it once, and would
avoid having undeclared properties because someone forgot.

I am very supportive the idea, although I see that it could be confusing
to some. Maybe the syntax needs to change, but the overall change is
much welcome.

Cheers

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

12 years ago by Martin Keckeis — view source — reply

unread

Am 08.08.2013 12:34 schrieb "Jordi Boggiano" j.boggiano@seld.be:

I'm not sure what problem this is really trying to solve, the
boilerplate
code you mention is very explicit and it is very clear to the reader
what
is being done. Each property documented with its type, purpose and
visibility in a common place for easy reference (at the top of the
class).
Each property that takes a value from the constructor assigned in the
constructor. Very clear.

I for one am pretty tired of writing this boilerplate in every second
class I write. Using dependency injection you end up having to write a
LOT of those usually, and constructors typically only contain assignments.

Adding a property means: declaring the property, adding the ctor arg,
adding the assignment in the ctor. You have to write the property name 4
times. With this RFC in it'd come down to writing it once, and would
avoid having undeclared properties because someone forgot.

I am very supportive the idea, although I see that it could be confusing
to some. Maybe the syntax needs to change, but the overall change is
much welcome.

The syntax itself i feel is self explaining. You read it and you know whats
going on.

But somehow i feel this is "wrong". Declaring visibility directly in the
arguments? This looks very scary to me. (Maybe only because i've never seen
it before...)

Other reasons against:
Phpdoc generator break
Code completion break
If constructor is not at the beginning never see a var declaration
Auto generation of set/get can be achieved with IDE..so its not much work

12 years ago by Matthieu Napoli — view source — reply

unread

Le 08/08/2013 13:11, Martin Keckeis a écrit :

Am 08.08.2013 12:34 schrieb "Jordi Boggiano" j.boggiano@seld.be:

I'm not sure what problem this is really trying to solve, the
boilerplate
code you mention is very explicit and it is very clear to the reader
what
is being done. Each property documented with its type, purpose and
visibility in a common place for easy reference (at the top of the
class).
Each property that takes a value from the constructor assigned in the
constructor. Very clear.

I for one am pretty tired of writing this boilerplate in every second
class I write. Using dependency injection you end up having to write a
LOT of those usually, and constructors typically only contain assignments.

Adding a property means: declaring the property, adding the ctor arg,
adding the assignment in the ctor. You have to write the property name 4
times. With this RFC in it'd come down to writing it once, and would
avoid having undeclared properties because someone forgot.

I am very supportive the idea, although I see that it could be confusing
to some. Maybe the syntax needs to change, but the overall change is
much welcome.

The syntax itself i feel is self explaining. You read it and you know whats
going on.

But somehow i feel this is "wrong". Declaring visibility directly in the
arguments? This looks very scary to me. (Maybe only because i've never seen
it before...)

Other reasons against:
Phpdoc generator break
Code completion break
If constructor is not at the beginning never see a var declaration
Auto generation of set/get can be achieved with IDE..so its not much work

I second the feeling about the syntax, there are too many disadvantages.
However the idea is excellent.

Maybe an alternative approach could do, here is a random suggestion (a
bit more verbose):

class MyClass {
public $foo;
protected $bar;

public function __construct($this->foo, $this->bar, $baz) {
// $this->foo and $this->bar are now set
$baz->doAnything(); // $baz is a standard parameter
}
}

This would even be compatible with an interface:

interface MyInterface {
function __construct($foo, $bar, $baz);
}

12 years ago by Leigh — view source — reply

unread

class MyClass {
public $foo;
protected $bar;

public function __construct($this->foo, $this->bar, $baz) {
// $this->foo and $this->bar are now set

This actually feels way more intuitive to me, it feels like you are
passing the parameter directly into that property.

With that style, why even limit it to the ctor?

function mySetter(array $this->items) {}

12 years ago by Robert Stoll — view source — reply

unread

I agree, that looks nice. It is still readable IMO and removes the
boilerplate code in the constructor (and setters and where needed)
This proposal seems much better to me because the class members $foo and
$bar are still in the same place as usual and the compiler does not generate
magic code as before.

-----Ursprüngliche Nachricht-----
Von: Leigh [mailto:leight@gmail.com]
Gesendet: Donnerstag, 8. August 2013 17:17
An: Matthieu Napoli
Cc: internals@lists.php.net
Betreff: Re: [PHP-DEV] RFC: constructor argument promotion

class MyClass {
public $foo;
protected $bar;

public function __construct($this->foo, $this->bar, $baz) {
// $this->foo and $this->bar are now set

This actually feels way more intuitive to me, it feels like you are
passing the parameter directly into that property.

With that style, why even limit it to the ctor?

function mySetter(array $this->items) {}

12 years ago by Michael Wallner — view source — reply

unread

class MyClass {
public $foo;
protected $bar;

public function __construct($this->foo, $this->bar, $baz) {
// $this->foo and $this->bar are now set

This actually feels way more intuitive to me, it feels like you are
passing the parameter directly into that property.

With that style, why even limit it to the ctor?

function mySetter(array $this->items) {}

Awww. No :-/

Sorry, I have no more words on that topic.

--
Regards,
Mike

12 years ago by Martin Keckeis — view source — reply

unread

Am 08.08.2013 15:12 schrieb "Matthieu Napoli" matthieu@mnapoli.fr:

Le 08/08/2013 13:11, Martin Keckeis a écrit :

Am 08.08.2013 12:34 schrieb "Jordi Boggiano" j.boggiano@seld.be:

I'm not sure what problem this is really trying to solve, the

boilerplate

code you mention is very explicit and it is very clear to the reader

what

is being done. Each property documented with its type, purpose and
visibility in a common place for easy reference (at the top of the

class).

Each property that takes a value from the constructor assigned in the
constructor. Very clear.

I for one am pretty tired of writing this boilerplate in every second
class I write. Using dependency injection you end up having to write a
LOT of those usually, and constructors typically only contain
assignments.

Adding a property means: declaring the property, adding the ctor arg,
adding the assignment in the ctor. You have to write the property name 4
times. With this RFC in it'd come down to writing it once, and would
avoid having undeclared properties because someone forgot.

I am very supportive the idea, although I see that it could be confusing
to some. Maybe the syntax needs to change, but the overall change is
much welcome.

The syntax itself i feel is self explaining. You read it and you know
whats
going on.

But somehow i feel this is "wrong". Declaring visibility directly in the
arguments? This looks very scary to me. (Maybe only because i've never
seen
it before...)

Other reasons against:
Phpdoc generator break
Code completion break
If constructor is not at the beginning never see a var declaration
Auto generation of set/get can be achieved with IDE..so its not much work

I second the feeling about the syntax, there are too many disadvantages.
However the idea is excellent.

Maybe an alternative approach could do, here is a random suggestion (a
bit more verbose):

class MyClass {
public $foo;
protected $bar;

public function __construct($this->foo, $this->bar, $baz) {
// $this->foo and $this->bar are now set
$baz->doAnything(); // $baz is a standard parameter
}
}

This would even be compatible with an interface:

interface MyInterface {
function __construct($foo, $bar, $baz);

}

Another thing which came into my mind:

I normall create a set/get method also to a construct parameter to
overwrite and unittest it...

So then i cant use this, because i construct i also use the set mehod to be
sure the var is assigned always the same (e.g. additional input check or
exceptions)

11 years ago by Dan Ackroyd — view source — reply

unread

Hi,

A little late to the game but here are some reasons why this RFC seems
be a bad idea to implement.

It makes it easier to write subtle bugs. e.g.

class A {
function __construct(protected $foo1 = 'A1', private $foo2 = 'A2') {
}
//other methods that use $this->foo2
}

class B extends A {
function __construct(protected $foo1 = 'B1', private $foo2 = 'B2') {
parent::__contruct();
}
//other methods that use $this->foo2
}

Class A and class B each have their own private copy of $foo2, but
share the single instance of property $foo1. Which is quite surprising
behaviour.

For my reading of the proposed RFC, I don't think it can safely be
used for sub-classes. i.e. if you wrote this code in the new proposed
syntax.

class SQLConnection extends DBConnection {}

class A {
function __construct(protected DBConnection $dbConnection){}
}

class B extends A {
function __construct(protected SQLConnection $dbConnection){}
}

That would be equivalent to writing this code in the current PHP syntax.

class A {
/** @var DBConnection */
protected $dbConnection;
function __construct(DBConnection $dbConnection){}
}

class B extends A {
/** @var SQLConnection */
protected $dbConnection;
function __construct(SQLConnection $dbConnection){
$this->dbConnection = $dbConnection;
}
}

But that's not what you would want. You don't want class B to
redeclare the property with a different type. So either the RFC would
have to implement some complex rules for typehinting, or people would
have to avoid using this syntax in sub-classes.

It hinders future ability to make new features on properties, such
as the RFC https://wiki.php.net/rfc/propertygetsetsyntax-alternative-typehinting-syntax
Cramming the declarations of properties into the constructor make it
almost impossible for any future RFC that wants to affect how
properties are declared.
Makes code harder to read. Currently the comments for a property
will be written directly above that property, so if you want to read
the comment for a property in any decent IDE you can i) Click on the
property ii) Move your eyes up one line, and you're reading the
comment for the property.

This RFC moves the comments further away and so you have to scan up
lines of code to figure out where the properties comment is. Even if
the other negative features of this RFC weren't present, making code
harder to read and maintain defeats any gain from a bit less typing.

Breaks current ecosystem of annotations. People who use annotations
for properties would have to rewrite their tools to support a new
syntax. Although that is sometimes necessary, it is a negative point
for this RFC.

To summarise, to me this RFC seems to have lots of negative features.
The RFC currently under vote
(https://wiki.php.net/rfc/automatic_property_initialization#vote)
doesn't save quite as much typing as this RFC would, but doesn't
suffer from any of the above negative features and so seems far better
to me.

If people prefer this RFC it'd be nice if there was some suggestions
of how to fix these issues.

cheers
Dan

Everyone -

Hi! Since this is my first post to this list, I'll introduce myself:
I'm an engineer who has been working on HipHop VM in New York for the last half year or so after a long time working at Microsoft on business software and services in multiple hemispheres.

I wanted to get the PHP community's opinion on this feature. See the draft RFC and referenced implementation below:

https://wiki.php.net/rfc/constructor-promotion

What do you all think?

Thanks,
Sean Cannella | Facebook Eng
If you are smart and persistent, you may find the truth. It is not always a reward.