Thursday, August 9, 2012

ok = ok ? ok : !ok

While working in CUS layer, I keep finding validate* methods that basically look like this:

public boolean validateWrite()
{
    boolean ok = true;
 
    ok = this.validateSomething() && ok;
    ok = this.validateSomethingElse() && ok;
    ...
    // johndoe 11.11.11 ->
    ok = this.validateWrittenByJohnDoe();
    // johndoe 11.11.11 <- 
 
    return ok;
}


As you can see, John forgot something: should validateWrittenByJohnDoe return true, the whole validateWrite would succeed, even if both validateSomething and validateSomethingElse failed. That may result in corrupted data.

There is only one case when skipping "&& ok" in the chain of validations is valid - when you explicitly set boolean variable to false:

    ok = checkFailed("@CUS1111");

Otherwise, either place an "ok" in front of your check, so that it would be skipped, if the result were false anyway:

    ok = ok && this.validateWrittenByJohnDoe();

or place it in the end, so all possible errors will be found and displayed in the infolog:

    ok = this.validateWrittenByJohnDoe() && ok;

Keeping all this in mind is especially important in AX 2012, where eventing is possible, as faulty event handlers on validate-method may lead to quite sneaky bugs.

1 comment:

  1. I sometimes see this one:

    if (...)
    ok = checkFailed("Some error") && ok;

    It doesn't really make sense to add the "&& ok" here, since you already have a false condition.

    If you are reviewing it is a good indication about how much you need to scrutinize the rest of the code.

    ReplyDelete