Extending Abstract Person Validator

I have the following code which is responsible for the validation in our app (just an example).

Based on the particular tab where user can add person to the database we need to perform additional validation (more fields in each tab).
Fields that always should be validated are name and surname so we provided an abstract class with subclasses that are specialized for each tab (i.e. Sourcing). Is it somehow possible to simplify this code by using some design patterns or something ?

abstract public class AbstractPersonValidator {

    public boolean isInvalid( Person person ) {
        if ( person.getName() == null || person.getName().length() < 3 )
            return true;

        if ( person.getSurname() == null || person.getSurname().length() < 3 )
            return true;

        return false;
    }
}

Extending:

public class SourcingPersonValidator extends AbstractPersonValidator {
    @Override
    public boolean isInvalid( Person person ) {
        if ( super.isInvalid( person ) )
            return true;

        if ( person.getTechnology() == null )
            return true;

        if ( person.getSource() == null )
            return true;

        return false;
    }
}

There are two suggestions I have for situations like this:

• double-negatives are bad

• use abstract methods for extensions

Let me explain. A method isInvalid(...) is really is not valid, and that can then be used in an expression !isInvalid(...) which is not is not valid... which is complicated. You should reverse the logic in your code, and call the method isValid(...)

That way, your code keeps a better 'read' to it.

The second issue is about how it is best to manage having some base functionality, and extending it for particular purposes. The way I prefer (and I am not sure if there is a formal 'pattern' for this process), is to have the base class do the actual work, and delegate to the specific class for the more fine-grained validation. Your base class would be (note the final on the isValid method....:

abstract public class AbstractPersonValidator {

    public final boolean isValid( Person person ) {
        if ( person.getName() == null || person.getName().length() < 3 ) {
            return false;
        }

        if ( person.getSurname() == null || person.getSurname().length() < 3 ) {
            return false;
        }

        return detailIsValid(person);
    }

    protected abstract boolean detailIsValid();

}

So, that class declares an abstract method that the child classes will need to implement, and the abstract method will do the child-specific checks:

public class SourcingPersonValidator extends AbstractPersonValidator {
    @Override
    protected boolean detailIsValid( Person person ) {
        if ( person.getTechnology() == null ) {
            return false;
        }

        if ( person.getSource() == null ) {
            return false;
        }

        return true;
    }
}

The advantage of doing things this way is that you don't run the risk of a bug where you forget to call super.

Note also that I added braces to your 1-liners. My experience suggests this makes for more reliable code when your code enters a maintenance cycle.

StackExchange Code Review Q#87189, answer score: 10