delight-im / PHP-Auth

Authentication for PHP. Simple, lightweight and secure.
MIT License
1.1k stars 235 forks source link

final class Auth {} - why is it final? #221

Open temuri416 opened 4 years ago

temuri416 commented 4 years ago

https://github.com/delight-im/PHP-Auth/blob/master/src/Auth.php#L22

Why is it final?

Undefined-Variables commented 4 years ago

PHP 5 introduces the final keyword, which prevents child classes from overriding a method by prefixing the definition with final. If the class itself is being defined final then it cannot be extended. Note: Properties and constants cannot be declared final, only classes and methods may be declared as final.

temuri416 commented 4 years ago

oh good lord

i know what final is.

the question is why that class was made final? I cannot extend it now.

Undefined-Variables commented 4 years ago

Id say its because that class contains all the functions that need to be kept most secure.

You can't extend a final class . But you can create an intermediate class, called Wrapper . The Wrapper will basically contain an instance of the original class and provide alternative methods to modify the state of the object according what you want to do

ocram commented 4 years ago

Thanks!

In general, we’re following the principle of designing for inheritance, instead of just opening up every single class to be extended and every method to be overridden:

Design and document for inheritance or else prohibit it

– Joshua Bloch: Effective Java

This is not to make anyone’s life harder, of course. Instead, it is to make things simpler for all of us, both developers and users of this library.

Access controls are there for a reason in object-oriented design, and removing final everywhere and changing private to protected (or even public) everywhere is not really the solution – in general, at least. This may vary between specific cases.

For most people and use cases, there shouldn’t really be a need to extend the classes here, to be honest. Do you disagree? What are the reasons that require subclassing and cannot be solved in a different way?

In many cases, it’s also true that composition instead of inheritance may be the better solution. Extension of classes is just one tool in your toolbox. It is not always the best tool to use.

What features are you trying to add? Perhaps these features or the discussion could be useful for all users of this library.

Another major issue why we’d rather not change the current design is that it allows for faster iteratations on the code of this library:

If we make the private methods and fields protected or public and allow for inheritance, these become part of the public API, and we have a breaking change and need to release a new major version whenever we change something in the signature of those methods or in the definition of the fields. Right now, developers working on this library can change those private members in any way they like and the library does not break for anybody who’s using it.

And if you just want final removed but don’t need any changes to the methods or fields, perhaps composition is indeed the better solution here.

See also:

Anyway, the decision is not set in stone. Let’s discuss this, and perhaps removing final (and possibly changing method or field visibilities) may be a good thing here.

luckdragon commented 2 years ago

I can think of a few reasons to want to extend it, 1, To make the "additional user information" part of an extended class rather than making it a general function in your application 2, I'd like to add additional statuses (such as Expired for accounts that are subscription based and their subscription has run out)

I'm sure there's many other reasons, but those are 2 that off the top of my head justify it NOT being final

eypsilon commented 2 years ago

I can think of a few reasons to want to extend it, 1, To make the "additional user information" part of an extended class

You can use the so-called Decorator pattern, to extend your Class with the Auth-Lib. It makes sure you can't overwrite the Auth-Lib while also allows you to easily switch to any Auth-Lib in no time, or even use different Libs at the same time.

eypsilon commented 2 years ago

Untested example, but you should get the idea. That's how i use the Lib.

use Delight\Auth\Auth;

class AuthDecorator
{
    protected static $authInstance;

    /**
     * @return void Instantiate Auth-Lib
     */
    public static function setInstance(): void
    {
        self::$authInstance = new Auth(...func_get_args());
    }

    /**
     * @return ?Auth Use Instance
     */
    public static function getInstance(): ?Auth
    {
        return self::$authInstance;
    }

    // extend here
}

// Usage
AuthDecorator::setInstance($db);

if ($auth = AuthDecorator::getInstance()) {
    print_r($auth->isLoggedIn());
}
eypsilon commented 2 years ago

I already was curious how to implement third Party Lib's without extending them. Turns out 5 methods can already get the job done pretty well. Great, I like this one already more than my currently used Solution :)

AuthDecorator.php