Boolean expression chains five or more operands
ID |
apex.maintainability.expression_too_complex |
Severity |
high |
Remediation Complexity |
medium |
Remediation Risk |
low |
Remediation Effort |
medium |
Resource |
Complexity |
Language |
Apex |
Description
Reports a boolean expression joining five or more operands with &&, or five or more operands with ||, at one level (fixed threshold: four operators). An expression that reaches four operators only by mixing && and || is reported only when the mixing is parenthesised, so that one level still carries five operands.
Rationale
Apex evaluates && and || left to right and short-circuits, so in a chain this long the later operands run for some records and not for others - and when one of them is a method call, a contains on a collection or a null-safe navigation, whether it runs at all depends on the order somebody happened to write. That is also what makes such a condition expensive to test: five operands need thirty-two combinations to cover, so in practice the test class covers two of them and the rest ship unverified. When the condition later needs one more clause, the person adding it has to re-derive the whole precedence by reading, and && binding tighter than || is the standard place that goes wrong.
The following code illustrates the pattern detected by this rule:
public class OpportunityGuard {
public Boolean isReadyToClose(Opportunity opp, User owner) {
// FLAGGED: Boolean expression chains five or more operands
return opp.Amount != null && opp.CloseDate != null && opp.StageName != 'Closed Lost' && opp.AccountId != null && owner.IsActive;
}
public void route(Case c, Boolean afterHours) {
// FLAGGED: Boolean expression chains five or more operands
if (c.Priority == 'High' || c.Origin == 'Phone' || c.IsEscalated || afterHours || c.Type == 'Outage') {
c.OwnerId = UserInfo.getUserId();
}
}
public Boolean needsReview(Account a, Boolean flagged) {
// FLAGGED: Boolean expression chains five or more operands
Boolean review = a.AnnualRevenue > 1000000 && a.Industry != null && (a.Type == 'Customer' || a.Type == 'Partner') && a.OwnerId != null && flagged;
return review;
}
public Boolean isComplete(Contact c) {
return c.FirstName != null && c.LastName != null && c.Email != null && c.Phone != null;
}
public Boolean isContactable(Contact c) {
return c.Email != null && c.Phone != null && (c.MailingCity != null || c.MailingStreet != null);
}
public Boolean hasOwner(Lead l) {
return l.OwnerId != null && l.Status != 'Converted';
}
public Boolean namedGroups(Opportunity opp, User owner) {
Boolean hasFigures = opp.Amount != null && opp.CloseDate != null;
Boolean isOpen = opp.StageName != 'Closed Lost' && opp.AccountId != null;
return hasFigures && isOpen && owner.IsActive;
}
public Boolean isStale(Account a, Boolean flagged, Boolean archived) {
// FLAGGED: Boolean expression chains five or more operands
return a.Industry != null && a.Type != null && a.OwnerId != null && a.ParentId != null && flagged && archived;
}
public Boolean anyChannel(Case c, Boolean escalated, Boolean afterHours, Boolean weekend) {
// FLAGGED: Boolean expression chains five or more operands
return c.Origin == 'Phone' || c.Origin == 'Email' || c.Origin == 'Web' || c.Origin == 'Chat' || escalated || afterHours || weekend;
}
}Remediation
Give the parts names: assign each group to a well-named Boolean local, or move the whole condition into a small private method whose name says what it decides, so the call site reads as one question. Where the operands are checks over the same record, a method on a wrapper or domain class keeps them together and makes each one testable on its own.