0

Should I validate the domain object properties when they are being set?

In this example I've got a User domain object from my model layer and currently am just validating the type and/or format of the passed in parameter before setting the property, because I have no clue of what should be validated when it comes to domain objects. Some examples would help me understand it.

Is this how I should validate domain object properties or shouldn't I validate them at all?

In the latter case I can then just remove every setter and getter and just make the domain object properties public so I can just interact with them directly.

class User
{
    private $id;
    private $firstname;
    private $lastname;
    private $email;
    private $password;
    private $registerDate;

    public function setId($id)
    {
        if (is_int($id)) {
            $this->id = $id;
        } else {
            throw new Exception('Parameter must be an integer.');
        }
    }

    public function setFirstname($firstname)
    {
        if (is_string($firstname)) {
            $this->firstname = $firstname;
        } else {
            throw new Exception('Parameter must be a string.');
        }
    }

    //{ etc setters }

    public function __get($property) {
        if (property_exists($this, $property)) {
            return $this->$property;
        }
    }
}
Kid Diamond
  • 2,232
  • 8
  • 37
  • 79

2 Answers2

0

Checking primitive types in a dynamic, weakly typed language seems like an overkill to me 99% of the time. It really doesn't matter if your $firstname is a proper string or not, it's going to be implicitly converted to one anyway when needed.

However, sometimes it does make sense to check whether a value of a primitive passed as an argument (not its type, mind you) meets some specific business logic constraints:

function squareRoot($number) {
    if(!is_numeric($number) || $number < 0)
        throw new \InvalidArgumentException("Positive number expected.");
    return sqrt($number);
}

That said, I would only use this kind of beaurocracy when absolutely necessary. If someone wants to pass a non-numeric string to a method named setNumber($number) with a doctype stating $number is a number, it's his problem if bad things happen in the result.

With methods expecting an object of a specific type (or implementing a specific interface) as arguments, use type hinting instead:

function foo(BarObject $bar) {
    // ...
}

This also works with arrays:

function doStuffWithArray(array $a) {
    // ...
}

Interestingly, there is no type hint that would allow both arrays and objects implementing \ArrayAccess interface, but that's just one of many PHP quirks.

lafor
  • 12,472
  • 4
  • 32
  • 35
0

My advice is to move validation logic from entities to class, which persists them, be it data mapper, repository or something else. Or better, separate validator class for each entity type / aggregate root.

Why? Separation of concerns.

Let's think about validation. I see two kinds of validators. First - type / range checking, for example, "Username must be string and between 5-50 chars". Second - domain specific validation, for example, "E-mail must be unique". Domain validation can be expensive and usually requires many supporting classes (services, data mappers, database adapters, etc).

Lauris
  • 1,085
  • 10
  • 16
  • In that case my domain objects would just be classes without setter and getters. They would only contain methods that return the state (`isLoggedIn()`, etc), and the properties would be set public, right? – Kid Diamond Jun 12 '14 at 09:41
  • This is up to you. I would keep getters and setters because when changing one field you may also need to change or calculate another. For example, setBirthday($birthday) { $this->birthday = $birthday; $this->horoscope = /* Calculate horoscope sign from $birthday */; } – Lauris Jun 12 '14 at 10:38
  • Well, in my cast they have no relations with each other. – Kid Diamond Jun 12 '14 at 11:27
  • Then public properties and optional domain specific entity methods will be totally OK. – Lauris Jun 12 '14 at 12:40
  • One more thing - isLoggedIn() belongs to some sort of AuthService class because there are many things involved, such as session, database access, request cookies, ... – Lauris Jun 12 '14 at 12:44