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.
I sometimes see this one:
ReplyDeleteif (...)
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.