On Fri, Sep 13, 2019 at 12:18 PM Rasmus Schultz <ras...@mindplay.dk> wrote:
> I'd like to address the examples - and why I think they don't demonstrate
> that this feature is really useful in practice.
>
> There are several examples similar to this one:
>
> class Car
> {
> public int $yearOfProduction;
> public string $vin;
> }
>
> This is either lacking a constructor to initialize the properties to the
> defined types - or it's lacking nullable type-hints. As it is, this class
> doesn't guarantee initialization according to it's own property type
> constraints.
>
> Assuming the fields of this entity are required, you would probably prefer
> to add a constructor - but then property initializers aren't really useful
> anymore. This seems to be the case for most of the examples.
>
Using initializers and a constructor and not mutually exclusive.
class User
{
public string $id;
public ?string $name;
public ?string $email;
public function __construct()
{
$this->id = bin2hex(random_bytes(16));
}
}
>
> Models often have other constraints besides just the type - in those cases,
> your models would likely have private/protected fields and setter-methods
> that implement those constraints. The visibility example demonstrates the
> use of a static factory method:
>
> class Customer
> {
> private string $name = '';
> protected ?string $email = null;
>
> public static function create(string $name, ?string $email = null): self
> {
> return new self {
> name = $name, // assign private property within the same class
> email = $email, // assign protected property within the same class
> };
> }
> }
>
> This may be fine for some use-cases - but many model types are only going
> to have one valid way to construct an instance, and constructors are the
> idiomatic way to do that. Unfortunately, this language feature works for
> the new-statement only, so a desire to use this language feature will drive
> architecture.
>
This example doesn't make a lot of sense. In this case, the `create` method
has no added benefit. You can just as well just use the constructor.
class Customer
{
private string $name;
protected ?string $email;
public function __construct(string $name = '', ?string $email = null)
{
$this->name = $name;
$this->email = $email;
}
}
Using constructor arguments isn't a great approach classes that have a
large number of properties, which is typically the case with data objects.
> All in all, I find this feature is useful or applicable only to a few,
> select patterns within the language - it isn't general enough.
> In my opinion, language features should be as general as possible - a
> feature like this "looks nice", being very abbreviated and clean-looking,
> and, as I believe the examples in the RFC itself demonstrates, this will
> provide the wrong kind of motivation to make what are, effectively,
> architectural decisions.
>
It seems like you consider the use of public properties as bad practice in
general. However, I do not think such a hard stance should be taken in the
ongoing discussion about public properties vs getters and setters. There
are valid arguments on both sides. If you don't use public properties, this
RFC will not affect you at all. If you do, there is a clear benefit in this
approach.
>
> Some models are immutable by design. Those cases don't seem to be well
> supported by this feature.
>
Immutable objects are not well supported in general in PHP. This RFC
doesn't affect it.
> My strong preference over this feature would be named parameters, which can
> provide the same abbreviated initializations, but works more consistently
> with the language, e.g. for all of the use-cases I cited above.
>
> It works for constructors:
>
> class Car
> {
> public int $yearOfProduction;
> public string $vin;
>
> public function __construct(int $yearOfProduction, string $vin) {
> if ($yearOfProduction < 1900 || $yearOfProduction > date("Y")) {
> throw new InvalidArgumentException("year of production out of range:
> {$yearOfProduction}");
> }
>
> $this->yearOfProduction = $yearOfProduction;
> $this->vin = $vin;
> }
> }
>
> $car = new Car({ yearOfProduction = 1975, vin = "12345678"});
>
> It works for static factory-methods:
>
> $car = Car::create({ yearOfProduction = 1975, vin = "12345678"});
>
> It works for models with private/protected fields, classes with accessors,
> classes with validations in constructors or factory-methods, and so on.
>
> In other words, it works more generally with all common patterns and
> practices - in many ways, it just seems like a better fit for the language.
>
I see how named parameters competes with this RFC. They are two different
things, that may both be implemented.
The need to define all properties as constructor arguments and then setting
them all isn't a great approach for classes with 10+ properties as it
really bloats the class. Also having property guard for public properties
that are only in the constructor isn't that great.
The common criticism against named parameters, is they create coupling to
> parameter-names. Object initializers create coupling to property-names - if
> you can live with that, I don't think named parameters or object
> initializers are very different in that regard.
>
You always have coupling to public property names. Public properties and
public methods are what make up the api of the class. Nothing in this RFC
changes that.
>
> The only problem I see with named parameters as an alternative to object
> initializers, is in terms of versioning - renaming an argument, today, is
> not a breaking change, and now it would be. That could be addressed, for
> example, by making named parameters explicit at the declaration site - for
> example, use curly braces in the declaration to designate named arguments:
>
> function makeCar({ int $yearOfProduction, string $vin }) {
> // ...
> }
>
> This way, adding named parameters is not a breaking change - it's an opt-in
> feature for those cases where it's useful and meaningful, but it's still
> applicable to all the patterns and cases that someone might want to use it
> for.
>
This is a discussion on itself. It has nothing to do with this RFC. Please
create a new RFC to discuss it.
>
> Named parameters is just one possible alternative - I'm just naming it for
> practical comparison, to demonstrate how some features may have more
> general applications than others.
> I'd prefer to see new features that work everywhere, all the time, for
> everyone - and for existing code. Rather than adding more features and
> syntax for very specific (even relatively rare) use-cases.
>
There is really nothing rare about the use of public properties. The RFC
shouldn't be invalidated because it doesn't benefit one particular
programming style, especially if that style is under heavy debate.
