CODEWITHSUNDEEP
phpinterface
geezmo
geezmo

Reputation: 161

Extends + implements, a question on good design principles

--- code changed ---

Suppose i have an interface and two classes, one of which extends the other one:

interface EncryptionModule {
    public function setup();

    public function encrypt($originalString);

    public function decrypt($encryptedString);
}

class GeneralOneWayHashingModule {
    public static $encryptionType = ALG_TYPE_HASH_FUNCTION;
    protected $salt;

    public function setSalt($salt) {
        $this->salt = $salt;
    }


    public function decrypt($encryptedString) {
        throw new Exception(__CLASS__ . ' ' . __LINE__ . ': cannot reverse an hash function ');
    }
}

class BCryptHashingModule extends GeneralOneWayHashingModule implements GeneralOneWayHashingModule {
    protected $cost;

    public function setup() {
        $this->cost = 10;
    }

    public function setCost($cost) {
        $this->cost = $cost;
    }

    public function encrypt($originalString) {
        return crypt($originalString, '$2a$' . str_pad($this->cost, 2, '0', STR_PAD_LEFT) . '$' . $this->salt);
    }
}

I wonder which one of the two classes should implement EncryptionModule interface.. the extending one (this is my choice)? Both classes, maybe with GeneralOneWayHashingModule declared as abstract and implementing all functions declared in EncryptionModule? Only the extended one?

Thanks.

Upvotes: 1

Views: 104

Answers (2)

soulmerge
soulmerge

Reputation: 75774

The question whether GeneralOneWayHashingModule should be abstract is unrelated to the other question: whether it should implement this interface. And the answer to both is: It depends on the Responsibilities of the classes, whether it is intended to be instantiated, and possibly other decisions.

I could give a better answer if I knew what each class is for and which methods it has.

EDIT after code-update

I see several issues here:

  • Why do you call the class …Module? I do not think it is a module, but a regular class. Its responsibility is encrypting stuff. I would rather call it Encryption (Names are extremely important to me, they will tell me what this class is for, even if I'm concentrating on something else - which is usually the case when I'm using a class)
  • As Carlos points out correctly in a comment to your question Encryption != Hashing and Encryption is by definition reverseable, whereas hashing is not, the two classes should not be extending each other.
  • What is the purpose of the setup() function? I think you added that function because you noticed that some sub-class needs it for its implementation. The question should rather be: Do all encryption functions have a set-up routine? Is it mandatory? I do not think so.
  • Another problem you have is that you want some of your encryption routines to use salt, whereas others do not seem to need one. Since salt is usually not re-used (i.e. you usually have a new salt for each encryption), it should not be a class member, otherwise your class has a state which is not what you want (i.e. $encryption can no longer encrypt() 2 strings in a row, its state has to be changed - setSalt() has to be called - for it to be able to encrypt the second string).
  • But the real question is: What do you need this class structure for? For testing/showcasing different algorithms or for storing/verifying passwords? If you just want to implement password verification, you are over-engineering, you could rather use something like this: 
    class User {
    public function encryptPassword($password, $salt) {
        // …
    }
    public function verifyPassword($password, $salt, $encryptedPassword);
        return $encryptedPassword == self::encryptPassword($password, $salt);
    }
}

Upvotes: 1

Simon
Simon

Reputation: 890

It depends of the other classes which inherit from GeneralOneWayHashingModule, if they all implements the EncryptionModule interface, then GeneralOneWayHashingModule should "implements" it too.

And, by the way, if GeneralOneWayHashingModule "implements" the EncryptionModule, then BCryptHashingModule "implements" it too, because it inherits of it (the "extends" key word).

Upvotes: 0

Related Questions