Make all middleware immutable about psr7-middlewares HOT 15 CLOSED

Just throwing my perspective in here (not that it is important at all).

From what I've learned about OOP design, immutability is only useful (and IMO a required practice) for business objects, entities, and their respective value objects, and data transfer objects; basically, anything that carries data over application boundaries. In that case, It would be expected that something like PSR's Request and Response interfaces are immutable.

When it comes to middlewares however, they are better defined as functional objects that interact with data to change it's state for other functional objects, and should not be immutable. In this case, immutability is more of a hindrance than a benefit.

The argument, albeit well argued, that the 'settings' of these middlewares can be edited can be addressed with a concern towards application design: If the application allows middleware or other functionality to alter settings on running middleware, then there is a boundary problem in the design of the application, and it is that which should be addressed rather than calling for immutability on functional objects.

@shadowhand I've actually read your article before this (and follow your work too), and it backs up my point, as you titled the article yourself:

Immutable Data Structures

Middleware is not a data structure, it is a functional object.

To tackle the argument about re-use that @schnittstabil makes: Yes, re-use is cool but is not the point of immutability. If you need to re-use object's with altered functionality, you should instantiate a new instance with either method that @oscarotero alluded to.

I hope I've not stirred the pot here, and hopefully added thought about this from an application design perspective.

from psr7-middlewares.

Why should the middleware be treated as inmutable?

from psr7-middlewares.

There's a number of articles out there about this, I'll tout one of my own: http://shadowhand.me/immutable-data-structures-in-php/ I think the best argument is:

An immutable object cannot be modified by accident.

Immutability has benefits and no (real) drawbacks.

from psr7-middlewares.

I got the point of inmutability in psr-7, but a middleware is just a callable, not an object shared between several applications. These methods are just sugar syntax, a way to provide configuration more friendly than using an associative array with settings.

I don't see benefits (or the need) of implementing inmutability here. But maybe I'm missing an use case to prove otherwise

from psr7-middlewares.

Immutability also improves reusability, e.g. in Slim:

$auth = Middleware::BasicAuthentication([
    'username1' => 'password1',
    'username2' => 'password2'
]);

$app->get('/', function ($req, $res) {
    // ...
})->add($auth->realm('Index Realm'));

$app->get('/login', function ($req, $res) {
    // ...
})->add($auth->realm('Login Realm'));

from psr7-middlewares.

Reusability looks cool, but method signatures (with~) has to be updated which can be non-BC change.

from psr7-middlewares.

There're many middlewares that immutability is confusing or problematic, mainly those that require other classes to work. For example, auraRouter, fastRoute or LeagueRoute: should the router instances be cloned? And whoops? Cors?, etc...
To reuse BasicAuthentication, for example, you can simply do this:

$app->add((clone $auth)->realm('Index Realm'));

Or configure your container or DI class to return a new instance each time:

$app->add($container->get('middleware:auth')->realm('Index Realm'));

from psr7-middlewares.

Typically only the thing being modified is cloned in the new instance. Collaborators are not.

from psr7-middlewares.

Ok, a simple way to add immutability without BC, is using a magic method:

    public function __call($name, $arguments)
    {
        if (stripos($name, 'with') === 0) {
            $method = substr($name, 4);

            if (method_exists($this, $method)) {
                $clone = clone $this;
                return call_user_func_array([$clone, $method], $arguments);
            }
        }

        throw new \BadMethodCallException(sprintf('Call to undefined method "%s"', $name));
    }

What do you think?

from psr7-middlewares.

@oscarotero As you already mentioned, things will become confusing/problematic, e.g. with Csp:

$csp = Middleware::csp($directives)
        ->withaddSource('img-src', 'https://ytimg.com'); // not immutable

This also shows that clone $csp is a bad idea, because as a programmer I don't know if $csp->csp is already instantiated or not.

Personally, I'm fine with the current approach, mainly because I almost always use DI.

from psr7-middlewares.

Ok, due the diversity of the middleware pieces, implementing immutability brings more problems than benefits, so it's better to leave the things as they are.

from psr7-middlewares.

Whoa whoa... that's a hasty conclusion... why wouldn't this be done as part of the next major release?

from psr7-middlewares.

Well, I can leave this issue opened for further discussion, waiting for a way to implement immutability clearly and intuitively.
But I still think that immutability is not necessary here.

from psr7-middlewares.

@designermonkey I think you make a good argument. Personally I don't see a downside to making everything immutable, but you're likely right that there is no particular benefit in this scenario.

from psr7-middlewares.

I'm agree with @designermonkey so I'll close this. Thanks everybody for the discussion. I've learned a lot.

from psr7-middlewares.