I have a class named MachineId. When an instance of this class is created, I want to make sure, that the constructor parameter is validated.

class MachineId
{
    
    private $uid;
    
    public function __construct(string $uid)
    {
        $this->uid = $uid;
    }
    
    public function validate() : bool
    {
        // check uid against specific format (by regex) and return true if valid and false if invalid
    }
    
}

My question is: When should I do the validation?

• Should I do the validation while I create an instance of MachineId? In this case it would make sense to make the validate method private. This would make it difficult to test it with Unittest.

// Validation WHILE instantiation
class MachineId
{
    
    private $uid;
    
    public function __construct(string $uid)
    {
        $this->uid = $uid;
        if($this->validate() === false) {
            // throw exception
        }
    }
    
    private function validate() : bool
    {
        ...
    }
    
}

• Should I do the validation before I create an instance of MachineId? This would make it possible to test the validate method by Unittest, requires that validation is called before creating the object. And it would make sense to make the method validate a static method (which I try to prevent).

// Validation BEFORE instantiation
class MachineId
{
    
    private $uid;
    
    public function __construct(string $uid)
    {
        $this->uid = $uid;
    }
    
    public static function validate(string $uid) : bool
    {
        ...
    }
    
}

if(MachineId::validate($uid) === false) {
    // throw exception
};
$machineId = new MachineId($uid);

• Should I do the validation after I create an instance of MachineId? This would make it possible to test the validate method by Unittest, requires that validation is called after creating the object.

// Validation AFTER instantiation
class MachineId
{
    
    private $uid;
    
    public function __construct(string $uid)
    {
        $this->uid = $uid;
    }
    
    public function validate() : bool
    {
        ...
    }
    
}

$machineId = new MachineId($uid);
if($machineId->validate() === false) {
    // throw exception
}

• Should I create an empty MachineId and then set the uid? In this case I would move the validation call into a setUid method. And again it would make sense to make the validate method private and so it is difficult to test.

// Validation in set method 
class MachineId
{
    
    private $uid;
    
    public function __construct()
    {
        $this->uid = null;
    }    


    public function setUid(string $uid)
    {
        $this->uid = $uid;
        if($this->validate() === false) {
            // throw exception
        }
    }
    
    private function validate() : bool
    {
        ...
    }
    
}

$machineId = new MachineId();
$machineId->setUid($uid);

• Should I create a validation class, that I also inject in the constructor?

// Validation with injecting a validation class

interface ValidatorInterface
{
    public function validate(mixed $value) : bool;
}

class MachineIdValidator implements ValidatorInterface
{
    public function validate($uid) : bool
    {
        ...
    }
}

class MachineId
{
    
    private $uid;
    
    public function __construct(string $uid, ValidatorInterface $validator)
    {
        if($validator->validate($uid) === false) {
            // throw exception
        }
        $this->uid = $uid;
    }    
}

$machineId = new MachineId($uid, new MachineIdValidator());

I run into this issue already many times and want to ask, if someone has a suggestion for a case like this. How to validate the string, without having a static method, but it is still testable and should not be forgotten?

My favorite solution would be to create a new instance of MachineId and provide the uid string as constructor argument. The uid should be validated and the validation method should also be testable.

CodePudding user response：

My favorite solution would be to create a new instance of MachineId and provide the uid string as constructor argument. The uid should be validated and the validation method should also be testable.

I think you are looking for something like:

// Validation WHILE instantiation
class MachineId
{   
    private $uid;
    
    public function __construct(string $uid)
    {
        $this->uid = $uid;
        if(MachineId::validate($uid) === false) {
            // throw exception
        };
    }
    
    public static function validate(string $uid) : bool
    {
        ...
    }    
}

Write as many tests as necessary to be confident that MachineId::validate is implemented correctly.

Recommended reading:

• Martin Fowler: Public vs Published Interfaces
• Alexis King: Parse, don't validate

CodePudding user response：

Unit testing is about testing the behaviour and not a method or a class.

I suggest keeping the validation within MachineId class. Doing so will make make it easier for your clients, as they won't have to worry about the validation again. My preferred approach here would be to make the constructor private and create a static factory method. You can then implement the validation in the static factory and create the object if the validation succeeds. Testing the validation behaviour then is as simple as testing the static factory itself.