1

--- 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.

3
  • 1
    This is an architecture and design question. These usually require more information than a couple of class names to answer.
    – Jon
    Sep 13, 2011 at 9:24
  • Apart from that everyone said, hashing is not encrypting. I guess your interface EncryptionModule would have both crypt() and decrypt(), and that's not possible with a OneWayHashing algorithm. So before implementing the interface, I would rename it to HashingModule Sep 13, 2011 at 9:34
  • You're right Carlos, i agree with you. But for my purposes it's not a problem to simply rise an exception when decrypt() is called on a module which implements a hashing function. Anyway: i'm editing my question to be more clear. Thanks!
    – geezmo
    Sep 13, 2011 at 9:41

2 Answers 2

1

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);
        }
    }
    
4
  • I changed the code - sorry for not have being more clear since the beginning. :-)
    – geezmo
    Sep 13, 2011 at 9:59
  • Sorry, but i think you just miss the point. I accept all of your observation, and thanks for giving me your point of view, but my question was not about hashing and/or encryption. I could provide an example with mammals, quadrupeds and dogs and my question would be the same.
    – geezmo
    Sep 13, 2011 at 13:01
  • @geezmo: But then my answer would be the same: You cannot solve such things generally, it always depends on the context - and on the context only. That's why I asked for more information in the first place. A good illustration of this heavy dependency on context is the question "Is a circle an ellipse?". The wikipedia entry presents a dozen solutions to this simple question: en.wikipedia.org/wiki/Circle-ellipse_problem
    – soulmerge
    Sep 13, 2011 at 13:38
  • I'd +1 your answer if i could, but i cannot do it (i'm new here and i do not have enough reputation). I'll do it as soon as i can, for your tips are precious. I'm still quite sure about the generality of my question ("if i use general classes for sharing code and interfaces for defining behaviours, interfaces must be implemented by general classes or by concrete implementations whose extend them?"), but i'll follow your links immediately.
    – geezmo
    Sep 13, 2011 at 14:19
0

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).

4
  • I changed the code - sorry for not have being more clear since the beginning. :-)
    – geezmo
    Sep 13, 2011 at 9:58
  • Then I think GeneralOneWayHashingModule should implements EncryptionModule and be an abstract class. This way you don't have to define encrypt in it.
    – Simon
    Sep 13, 2011 at 10:26
  • Thanks Simon! Actually, the purpose of GeneralOneWayHashingModule is to avoid code repetition - it actually shares code with classes whose extend it. The behaviuor that concrete classes should follow is entirely described by the interface. GeneralOneWayHashingModule class could implement the interface, but declaring the encrypt() method as abstract in GeneralOneWayHashingModule class is not the same as implementing the interface with BCryptHashingModule directly?
    – geezmo
    Sep 13, 2011 at 14:31
  • Not exactly, if you're doing it, any GeneralOneWayHashingModule subclass will not by automagically an EncryptionModule. You have to keep in mind that abstractions are particularly useful when you don't know the "exact" (called "dynamic") type of an object. For instance, a function could take a GeneralOneWayHashingModule as argument, if the GeneralOneWayHashingModule implements the EncryptionModule interface, then you don't have to know what subclass of GeneralOneWayHashingModule it is to use the encrypt and decrypt methods.
    – Simon
    Sep 14, 2011 at 4:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.