When you have some non-trivial boolean statement used in if
like:
private void evict(Data data) {
// ...
if (data.getDate() >= MATURITY_DATE && data.type == DataType.UNDEF) {
// do the eviction
}
}
To make it easier to read and maintain, it’s a good idea to extract it to a local variable or method:
private void evict(Data data) {
// ...
boolean isEligibleForEviction = data.getDate() >= MATURITY_DATE
&& data.type == DataType.UNDEF;
if (isEligibleForEviction) {
// do the eviction
}
}
or
private void evict(Data data) {
// ...
if (isEligibleForEviction(data)) {
// do the eviction
}
}
boolean isEligibleForEviction(Data data) {
return data.getDate() >= MATURITY_DATE && data.type == DataType.UNDEF;
}
Notice how it’s easier to understand what’s happening - and it’s more obvious without even adding a single line of comment.
The first approach (local variable) has the advantage of being very simple and in-place; no need to pass any arguments. The second approach (method) has the advantage of being easy to test and we all love easy testable code, right?
Additionally – because business guys tend to change requirements faster than .js frameworks are created – it’s easier to apply some comment why the logic was changed:
private void evict(Data data) {
// ...
if (isEligibleForEviction(data)) {
// do the eviction
}
}
// On-purpose allow all data to be eligible for eviction (JIRA-1234)
boolean isEligibleForEviction(Data data) {
return true;
}
This might seem redundant - to put the JIRA number (or any other tracking system ID) as a comment to the method directly in the code - everything is in the VCS, right?
However, I had many cases in which this simple comment saved me and my colleagues a lot of time. In large distributed teams it’s easy to get lost in constantly changing requirements and such type of comment shows the exact intent of the author to code it this way.
PS. It might also be a good idea to create a Predicate (Guava or JDK) from such method or variable and reuse it e.g. in some collections filtering or stream operations.
PPS. In both cases (variable boolean and boolean method) take care of splitting the condition into smaller pieces. If you code all
in one conditional if
then you’ll benefit from the short-circuit evaluation. If you’ll
fall too much into breaking the condition into multiple simpler ones, then short-circuit will not kick in. This might be not only
a performance efficiency problem but also a code correctness one:
boolean isMatured = data.getDate() >= MATURITY_DATE;
// this will always be evaluated even if isMatured is false
boolean isUndefined = data.type == DataType.UNDEF;
boolean isEligibleForEviction = isMatured && isUndefined;
or
boolean isNotNull = data != null;
// this will always be evaluated even if data is null => NPE
boolean isUndefined = data.type == DataType.UNDEF;
boolean isEligibleForEviction = isMatured && isUndefined;