On Tue, Mar 17, 2020, at 1:07 PM, Máté Kocsis wrote:

> At this point I'd like to repeat one of my previously mentioned arguments
> for the feature:
> if we have an immutable PSR-7 that is used all over the PHP ecosystem, and
> we also have
> lots of people who wish for more type-level strictness by using immutable
> DTOs or VOs,
> why don't we provide a proper way to achieve immutability? If there is such
> a need
> then I think we should react. Even more so, because the same feature is
> available in other
> languages where we usually copy from, so it wouldn't even be without
> precedents.
> (Just to be clear: I don't think that we have to copy everything from Java
> or C#. :) )

I don't have voting rights yet, but I would also vote no as is precisely 
because of the PSR-7 type case, along the same lines that Nicolas is arguing.

Currently, a PSR-7 implementation does something like this (assuming it were 
updated to typed properties):

class Response implements ResponseInterface {

  private int $statusCode;

  public function getStatusCode : string {
    return $this->statusCode;
  }

  public function withStatusCode(int $code): self {
    $new = clone($this):
    $new->statusCode = $code;
     return $new;
  }
}

Now repeat that for about 10 different properties across 3 different objects, 
and sometimes with multiple variations on the with*() method.

Allowing the status code to be declared public-readonly would change it to this:

class Response implements ResponseInterface {

  public readonly int $statusCode;

  public function withStatusCode(int $code): self {
    $new = new static(
      $code, 
      $this->headers, 
      $this->protocolVersion, 
      $this->body
     );
     return $new;
  }
}

And that's for Response, which is the simplest of the message objects in PSR-7.

That is, you get to skip the trivially easy method (a positive) in return for 
having a longer and more involved with*() method, which you already have more 
of to begin with (a negative), and for forcing you to have a constructor that 
takes every possible property (a negative).  

That is not a net win.

Thinking about it, Uri is possibly an even better example as it has 8 
properties:

class Uri implements UriInterface {

    private string $scheme;

    private string $authority;

    private string $userInfo;

    private string $host;

    private int $port

    private string $path;

    private array $query;

    private string $fragment;

    // These methods could all go away, but they're the simple ones:

    public function getScheme() {
        return $this->scheme;
    }

    public function getAuthority();

    public function getUserInfo();

    public function getHost();

    public function getPort();

    public function getPath();

    public function getQuery();

    public function getFragment();

    // Whereas these turn from this:

    public function withScheme(string $scheme) {
      $new = clone($this);
      $new->scheme = $scheme;
      return $new;
    }

    // In this:

    public function withScheme(string $scheme) {
      $new = static(
        $scheme, 
        $this->authority, 
        $this->userInfo, 
        $this->host, 
        $this->port, 
        $this->path, 
        $this->query, 
        $this->fragment
       );
      return $new;
    }

   // And doing so for all of these methods, but slightly differently:

    public function withUserInfo($user, $password = null);

    public function withHost($host);

    public function withPort($port);

    public function withPath($path);

    public function withQuery($query);

    public function withFragment($fragment);
}

So switching it to use readonly properties as proposed here, would involve:

* More complex methods
* A mandatory 8 parameter constructor
* Avoiding the trivial methods in exchange for the mandatory methods being 
longer.

Honestly... that's not usable for value objects.  I love value objects, I love 
immutable values, and I would find this property useless in its current form.

Also, I just realized... properties cannot be defined in interfaces.  That 
means a PSR interface (or any other interface) could not require that they be 
defined, which means they cannot be made part of a contract.  That renders 
readonly properties as defined here unusable by FIG entirely.

Requiring that properties be typed to be readonly is fine with me.  But the way 
it renders cloning unusable for evolvable objects makes evolvable objects 
considerably harder to use and to standardize, not easier.

And this property co-existing with some other way of achieving the same goal 
(whether that's something like Nicolas's proposal or something entirely 
different) would only lead to confusion and people doing one, realizing they 
should have done the other, and having more BC breaks now in their own code.

I appreciate the work that Mate has put into this proposal, but as is I have to 
encourage people to vote no.

--Larry Garfield

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to