Return $this directly without using clone class instance

[peter279k]

As title, please consider the following sample code firstly:

public function withCookieParams(array $cookieParams) : ServerRequestInterface
{
     $clone = clone $this;
     $clone->cookieParams = $cookieParams;
     return $clone;
}

How about using the following code?

public function withCookieParams(array $cookieParams) : ServerRequestInterface
{
     $this->cookieParams = $cookieParams;
     return $this;
}

I think using the clone to clone $this instance is not necessary because it should override the methods/attributes on $this class instance.

[fenric]

Hello @peter279k :)

This method MUST be implemented in such a way as to retain the
immutability of the message, and MUST return an instance that has the
updated cookie values.

Source: https://www.php-fig.org/psr/psr-7/#321-psrhttpmessageserverrequestinterface

/**
 * Return an instance with the specified cookies.
 *
 * The data IS NOT REQUIRED to come from the $_COOKIE superglobal, but MUST
 * be compatible with the structure of $_COOKIE. Typically, this data will
 * be injected at instantiation.
 *
 * This method MUST NOT update the related Cookie header of the request
 * instance, nor related values in the server params.
 *
 * This method MUST be implemented in such a way as to retain the
 * immutability of the message, and MUST return an instance that has the
 * updated cookie values.
 *
 * @param array $cookies Array of key/value pairs representing cookies.
 * @return static
 */

[peter279k]

Hi @fenric, thank you for your reply.

You explain that makes sense.

How about using the following code:

...
	public function withServerParams(array $serverParams) : ServerRequestInterface
	{
                if ($this->serverParams === $serverParams) {
                    return $this;
                }

		$clone = clone $this;
		$clone->serverParams = $serverParams;
		return $clone;
	}
...

We can check whether the params are different before setting new params.

And it can prevent the unnecessary setting params and cloning instance work.

It has the same issues for other methods on this package.