Nyholm / psr7

A super lightweight PSR-7 implementation
MIT License
1.16k stars 75 forks source link

Remove final keyword #169

Closed Nyholm closed 3 years ago

Nyholm commented 3 years ago

I don't think there is any benefit to remove the final keyword. All the good software scientists and best practises says that you should never efter extend these value objects.

However, some people insists on writing code that I don't approve of and from their perspective the final keyword is just annoying.

This PR does not change the concept, but technically it allows some people to write the code they want to write even tough I consider it suboptimal.


Edit: When I read this description again, it looks that Im condescending. Im really not. Im trying to be pragmatic.

Zegnat commented 3 years ago

I also really dislike this, because extending a PSR implementation tends to people relying on the extended version, which in turn means they have just limited themselves to never be able to switch implementations. A problem that does not happen if you follow a decorator pattern. The entire point is interoperability. I almost feel this is just helping someone else get into trouble down the line.

I have been thinking about ways to help people build decorators, and wonder if it might be better to offer some sort of abstract class to extend that implements all the methods. E.g. we'd be providing the abstract class called ADecoratorBase from this example:

interface AnInterface { // Imagine this is a PSR-7 value object interface.
    public function action(): bool;
}

final class AnImplementation implements AnInterface { // Imagine this is our implementation of said interface.
    public function action(): bool {
        return true;
    }
}

abstract class ADecoratorBase implements AnInterface { // Imagine this is a "decorator" we provide.
    protected AnInterface $base;

    public function action(): bool {
        return $this->base->action();
    }
}

class MyDecorator extends ADecoratorBase { // Now all a user needs to do to decorate is this.
    public function __construct(AnInterface $base)
    {
        $this->base = $base;
    }
}

$obj = new MyDecorator(new AnImplementation());
assert($obj->action() === true, 'Accessing expected method from base.');

Alternatively shipping something like the Grav traits that @mahagr linked to in https://github.com/Nyholm/psr7/issues/139#issuecomment-567828427.

Just wondering if we can do some sort of educative good by doing that, rather than just dropping the final keyword that we both believe as correct for this library :thinking:

mahagr commented 3 years ago

I have to say that extending class also sounds tempting as it would allow customization without too much work. This is also where the traits come -- it will save a lot of work. The reason why I ended up using traits was to be able to inject the PSR-7 interface into any (existing) class.

I think it's also a good idea to separate decorator/traits from this library in order to prevent fixating code into a single library.

Nyholm commented 3 years ago

@Zegnat, Yeah, but looking at the implementations, most methods just have a few lines, I don't think it make much sense to also add a bunch of abstract classes.

What if we add a page in the documentation to better explain why extending classes are a bad idea. We can also show what one should do instead. The @final comment can link to that page.

Nyholm commented 3 years ago

The PR is updated

Nyholm commented 3 years ago

Thank you.

Let me improve the docs with some more references.

frodeborli commented 3 years ago

First of all; I like the quality of your work - which is why I decided to use your PSR-7 implementation. Unfortunately, I have to drop it or fork it due to the @final keyword.

The PSR does not define the signature for the __construct method. Since your class prohibits overriding the constructor - thanks to the @final keyword, I don't think your implementation complies

I'm sad to learn that you have decided to mark your class with @final, for political reasons. Reason seems to be that you think I should change my style of programming because you think composition is awesome...