Class to verify if a rectification is upgradable Today I encountered something that I have been wondering about in the past a few times before. How can I refactor a method that has this format:

private boolean isRectificationUpgradable(Rectification rectification) {

    final boolean validType = rectification.getType() == RectificationType.VH
            || rectification.getType() == RectificationType.COL;

    if (!validType) {
        return false;
    }

    if (rectification.getStatus() != RectificationStatus.IN_PROGRESS &amp;&amp; rectification.getStatus() != RectificationStatus.OPEN) {
        return false;
    }

    if (rectification.getVehicle() == null) {
        return false;
    }

    final List&lt;CarPassCertificate&gt; certificates = rectification.getVehicle().getCertificates();

    if (CollectionUtils.isEmpty(certificates)) {
        return false;
    }

    CarPassCertificate validCertificate = getValidCertificate(rectification);

    if (validCertificate == null) {
        return false;
    }

    return true;
}


When a rectification is created it has the status "open". An employee indicates that he started to handle it by putting it in the status "in progress".

The upgradable in this context means that sometimes the creator chose a certain type for the rectification, when in fact he could have picked a more accurate other type. In this case someone in the application can "upgrade" this rectification to the more specific type.

In code, this "upgraded" type is not modeled as another class as it is not really an upgrade, it always stays the same Rectification class and some shared additional fields are set to make it a bit more specific.

I think it is pretty readable but as more validations are added it can become very long. Any ideas?
This looks as if it is part of the upgrade code itself.  I think you have too low a level of abstraction in here and you are also mixing different concerns.  Also, your workflow just seems fragile.

Cars and certificates

You check that there is a car, then you check that it has any certificates, then you pass the rectification to getValidCertificate.  Either you only need to pass the certificate list or you don't need to do the "has car/certificates" check yourself, since getValidCertificates clearly has to do that itself to be able to return a CarPassCertificate (or null).

(Since starting this question, we've discussed this issue in comments and I see you've agreed that only getValidCertificates needs to do the check.  Nice.)

Workflow

How can a rectification be both open and in progress and yet not even have a car?  Even if it is legitimate to have one open with no car, I can't see how it is valid for an in-progress task.  I really think your type hierarchy may not properly model your workflow.

If a rectification may exist for a period without a car (let's think of it as a rectification request) but


Rectifications can only be actioned once a car is added
Cars are not removed from Rectifications once added


then you should have two different classes to represent them. The first type should not have a car field at all.  The second should not be creatable without being passed a non-null car object. Depending on the rest of your code, the second class could be a subclass of the first (with the addition of a car field amongst other things) or share an interface with the first (preferred option of those two) or the request could be an entirely separate class, replaced with a car-containing rectification by a factory.

Either way, isRectificationUpgradable only need specify the second type as input. And immediately a whole category of errors is eliminated.  There is no need to check for the existence of a car when it is guaranteed to be present.

If you do it this way, you never have to check for the presence of a real car.  Any method that depends on the existence of a car simply has to specify the car-owning type.

I would not be surprised if other stages in the lifecycle of rectifications can be treated this way.  If you create a common rectification interface but 


only the OpenRectification class/interface (which has no employee field) has a toAssigned method which requires an employee object and returns an assignedRectification object
only the assignedRectification type has changeAssignedEmployee method and toInProgress methods
only thetoInProgress type has methods which can be used to (for example) log work done


then many more categories of error evaporate.