On Tue, May 5, 2020 at 10:27 PM Larry Garfield <la...@garfieldtech.com> wrote:
> On Tue, May 5, 2020, at 7:35 AM, Nikita Popov wrote: > > > > Performing validation when the getAttributes() call is performed does > sound > > reasonable to me. We can also add a class flag to perform this validation > > only once (if it is successful), so the cost doesn't have to be paid by > > every single getAttributes() consumer. > > > > For the purpose of this RFC, I've now updated it to say that attributes > > will be applied to both properties and parameters ( > > https://wiki.php.net/rfc/constructor_promotion#attributes), but with an > > explicit note that we should change the behavior prior to the PHP 8 > release > > if it turns out to be problematic, e.g. with regards to an attribute > > validation feature. I think this is something of a detail, and we'll be > > mostly fine whichever way we chose, but it's hard to say right now which > > option is best, in particular with regard to future attributes > improvements. > > > > Regards, > > Nikita > > Thinking on it further, there's something sitting in the back of my head > that's bothering me. This helped me flesh it out. > > I like promotion, and think it will be great for simplifying code. > However, as attributes show as we load more bells and whistles onto > properties (even if they are desirable and useful bells and whistles) the > level of conflict is going to get larger. > > Consider, today we have: > > class Point { > protected int $x; > protected int $y; > > public function __construct(int $x, int $y) { > $this->x = $x; > $this->y = $y; > } > } > > And this is needlessly verbose. With promotion, that simplifies to: > > class Point { > public function __construct(protected int $x, protected int $y) {} > } > > And this is good. There seems little dispute about that. > > But then we add attributes, and it starts to get a little odd: > > class Point { > public function __construct( > <<Positive>> > protected int $x, > > <<Positive>> > protected int $y, > ) {} > } > > And has the possible confusion between parameter attributes and property > attributes. > > But... there's plenty of other things that may get added to properties. > We've discussed the "readonly" property and the more robust assymetric > visibility. What would that look like? > > class Point { > public function __construct( > <<Positive>> > int $x { > public get, > private set, > }, > > <<Positive>> > int $y { > public get, > private set, > }, > ) {} > } > > And that's now an awful lot of metadata to shove into, technically, a > function signature. And if accessor methods ever happen: > > class Point { > public function __construct( > <<Positive>> > int $x { > public get () { > return $this->x; > }, > private set($new) { > if ($new < 0) { > throw new Exception(); > } > $this->x = $new; > }, > }, > > <<Positive>> > int $y { > public get () { > return $this->y; > }, > private set($new) { > if ($new < 0) { > throw new Exception(); > } > $this->y = $new; > }, > }, > ) {} > } > > And now we have methods defined inside a function signature, and that's > just bizzarro. It also means at least 4 indent levels before you even get > to the body of the methods. > > And there's probably other things that could get attached to properties at > some point I'm not thinking of that someone will suggest. > > So I have to wonder, are we painting ourselves into a hole? Most of the > uses I can think of for extra-metadata-properties are... overlapping with > the case where you want to have constructor promotion. That means "well if > you need to do fancy stuff just don't use promotion" becomes a > non-sustainable answer, because the cases where you want both will increase > rather than be an edge case. > > Is there a way we can build in an escape hatch for ourselves to avoid > these issues, both attributes and for future extension? > > First idea off my head, which may or may not be good: > > Allow an alternate keyword to fill in the body of the constructor, but NOT > fill in the property. You still define the property yourself and put all > the extra metadata there. To wit: > > class Point { > <<Positive>> > int $x { > public get () { > return $this->x; > }, > private set($new) { > if ($new < 0) { > throw new Exception(); > } > $this->x = $new; > }, > }, > > <<Positive>> > int $y { > public get () { > return $this->y; > }, > private set($new) { > if ($new < 0) { > throw new Exception(); > } > $this->y = $new; > }, > }, > > public function __construct(promote int $x, promote int $y) {} > } > > That eliminates 2 of the 4 places you'd have to repeat the name instead of > 3, but means the constructor isn't burdened with having to hold a > potentially non-trivial amount of metadata. It also resolves the attribute > question as splitting it up like this would let you do "one or the other" > (property vs parameter). > > That may not be the best way, but I'm slightly uneasy with a path that > could lead to 30 lines of property metadata squeezed into a constructor > signature definition. :-) > Hey Larry, At least for me, there's a pretty clear line on what is acceptable to use with constructor promotion and what isn't: That line has been crossed when your property declarations started to include their own {} blocks. Constructor promotion is a short-hand notation for the cases where the constructor boilerplate is much larger than the actual functionality. If you have 100 lines of property declarations due to heavy accessor use, then I think the value proposition of constructor promotion converges to zero, because the constructor boilerplate is no longer a dominating factor. Regarding your last example, something similar to this has already been proposed (and declined) in a slightly different form in https://wiki.php.net/rfc/automatic_property_initialization: class Point { public int $x { ... }; public int $y { ... }; public function __construct(int $this->x, int $this->y) {} } Regards, Nikita
