mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
Add case true of pattern for long condition chains
Follow-up from PR review: nested if is the right answer for two or three dependent conditions, but past that the nesting becomes the problem. AL's case statement is the flat alternative — the control statements documentation states a value set "must be an expression or a range" and that the first matching value set executes, so case true of / case false of accept boolean expressions and stop at the first match. That is the laziness the boolean operators do not provide. Adds case-true-of-for-long-condition-chains.md with good/bad AL companions: case false of for guard chains where every condition must hold, case true of for first-match dispatch. The bad sample shows both failure shapes — a five-level if ladder, and the worse escape of collapsing it into an and chain, which trades nesting for a real defect. Cross-links both articles, and adds the threshold to the short-circuit article's Best Practice so following it does not lead to a deep ladder. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
1496e72b7b
commit
63bd69d7e5
4 changed files with 108 additions and 2 deletions
|
|
@ -15,7 +15,7 @@ AL gives no short-circuit (lazy) evaluation guarantee for `and`, `or`, and `xor`
|
|||
|
||||
## Best Practice
|
||||
|
||||
Split a condition into nested `if` statements whenever one operand is only safe or only worth evaluating once another has passed. The guarding or cheapest condition goes in the outer `if`, the dependent or expensive one in the inner `if`, which makes the dependency explicit and keeps the expensive operand off the path that would have been skipped. Where there is no `else` branch, nesting is a pure win; where there is one, extract the conditions into a helper procedure that exits early instead. Keep `and` and `or` for operands that are independently safe and cheap — in-memory field comparisons, enum tests, bound checks — where combining them reads better and costs nothing.
|
||||
Split a condition into nested `if` statements whenever one operand is only safe or only worth evaluating once another has passed. The guarding or cheapest condition goes in the outer `if`, the dependent or expensive one in the inner `if`, which makes the dependency explicit and keeps the expensive operand off the path that would have been skipped. Where there is no `else` branch, nesting is a pure win; where there is one, extract the conditions into a helper procedure that exits early instead. Where the chain runs past about three conditions, stop nesting and use a `case` statement instead — see `case-true-of-for-long-condition-chains.md`. Keep `and` and `or` for operands that are independently safe and cheap — in-memory field comparisons, enum tests, bound checks — where combining them reads better and costs nothing.
|
||||
|
||||
See sample: `boolean-operators-do-not-short-circuit.good.al`.
|
||||
|
||||
|
|
@ -27,4 +27,4 @@ See sample: `boolean-operators-do-not-short-circuit.bad.al`.
|
|||
|
||||
## See also
|
||||
|
||||
`microsoft/knowledge/performance/apply-guards-before-get.md` covers the related ordering rule for statements rather than operands.
|
||||
`case-true-of-for-long-condition-chains.md` covers what to do when nesting the conditions would go more than about three levels deep. `microsoft/knowledge/performance/apply-guards-before-get.md` covers the related ordering rule for statements rather than operands.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,28 @@
|
|||
codeunit 50543 "Perf Sample CaseChain Bad"
|
||||
{
|
||||
procedure IsShippableLine(SalesLine: Record "Sales Line"): Boolean
|
||||
var
|
||||
Item: Record Item;
|
||||
begin
|
||||
// Five levels of nesting to sequence five guards. The evaluation order is
|
||||
// carried by indentation alone and the body drifts steadily right.
|
||||
if SalesLine.Type = SalesLine.Type::Item then
|
||||
if SalesLine."No." <> '' then
|
||||
if SalesLine."Qty. to Ship" > 0 then
|
||||
if Item.Get(SalesLine."No.") then
|
||||
if not Item.Blocked then
|
||||
exit(true);
|
||||
exit(false);
|
||||
end;
|
||||
|
||||
procedure IsShippableLineCollapsed(SalesLine: Record "Sales Line"): Boolean
|
||||
var
|
||||
Item: Record Item;
|
||||
begin
|
||||
// The wrong escape from the ladder: flattening it into 'and' trades the
|
||||
// nesting for a defect, because every operand is still evaluated. Item
|
||||
// fields are read even when the Get failed.
|
||||
exit((SalesLine.Type = SalesLine.Type::Item) and (SalesLine."No." <> '') and
|
||||
(SalesLine."Qty. to Ship" > 0) and Item.Get(SalesLine."No.") and (not Item.Blocked));
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,48 @@
|
|||
codeunit 50542 "Perf Sample CaseChain Good"
|
||||
{
|
||||
procedure IsShippableLine(SalesLine: Record "Sales Line"): Boolean
|
||||
var
|
||||
Item: Record Item;
|
||||
begin
|
||||
// 'case false of' evaluates the value sets in order and stops at the first
|
||||
// match, so each condition is reached only when the previous one passed —
|
||||
// and Item.Get is never called for a non-item line.
|
||||
case false of
|
||||
(SalesLine.Type = SalesLine.Type::Item):
|
||||
exit(false);
|
||||
(SalesLine."No." <> ''):
|
||||
exit(false);
|
||||
(SalesLine."Qty. to Ship" > 0):
|
||||
exit(false);
|
||||
Item.Get(SalesLine."No."):
|
||||
exit(false);
|
||||
(not Item.Blocked):
|
||||
exit(false);
|
||||
end;
|
||||
exit(true);
|
||||
end;
|
||||
|
||||
procedure FindOpenDocumentType(CustomerNo: Code[20]): Text
|
||||
begin
|
||||
// 'case true of' stops at the first condition that holds, so the later
|
||||
// lookups never run once an earlier one matched.
|
||||
case true of
|
||||
HasOpenDocument(CustomerNo, "Sales Document Type"::Quote):
|
||||
exit('Quote');
|
||||
HasOpenDocument(CustomerNo, "Sales Document Type"::Order):
|
||||
exit('Order');
|
||||
HasOpenDocument(CustomerNo, "Sales Document Type"::Invoice):
|
||||
exit('Invoice');
|
||||
end;
|
||||
exit('None');
|
||||
end;
|
||||
|
||||
local procedure HasOpenDocument(CustomerNo: Code[20]; DocumentType: Enum "Sales Document Type"): Boolean
|
||||
var
|
||||
SalesHeader: Record "Sales Header";
|
||||
begin
|
||||
SalesHeader.SetRange("Document Type", DocumentType);
|
||||
SalesHeader.SetRange("Sell-to Customer No.", CustomerNo);
|
||||
exit(not SalesHeader.IsEmpty());
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,30 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [case-statement, case-true-of, nested-if, condition-chain, guard, lazy-evaluation, nesting-depth]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use case true of for long chains of dependent conditions
|
||||
|
||||
## Description
|
||||
|
||||
Because AL gives no short-circuit guarantee for `and` and `or`, a chain of conditions that must be evaluated in order has to be sequenced with nested `if` statements — and past three conditions the nesting itself becomes the problem: the body drifts right, the order of evaluation is carried by indentation alone, and any shared failure path is repeated at every level. AL's `case` statement is the flat alternative. Its value sets "must be an expression or a range", so `case true of` and `case false of` accept arbitrary boolean expressions, and the statement "is evaluated, and the first matching value set executes the associated statement" — evaluation stops at the first match, which is exactly the laziness the boolean operators do not provide.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Sequence two or three dependent conditions with nested `if`. Beyond that, switch to `case`: use `case false of` for a chain of guards where every condition must hold, listing the failure action per condition and letting control fall past `end` when all pass; use `case true of` for first-match dispatch, where each later probe runs only if the earlier ones did not match. This keeps every condition at one indentation level, makes evaluation order explicit rather than implied by nesting, and preserves the stop-at-first-match behaviour. It also aligns with the AL programming convention that more than two alternatives belong in a `case` statement rather than an `if-then-else`.
|
||||
|
||||
See sample: `case-true-of-for-long-condition-chains.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
An `if` ladder four or more levels deep whose only purpose is sequencing guards. Detection: a chain of nested `if` statements with no `else`, each condition guarding the one below it, terminating in a single action or `exit`; or the same `exit`/`error` duplicated at every level of such a chain. The second, worse form is collapsing that ladder into one `and` chain to escape the nesting — that trades indentation for a real defect, because the operands are still all evaluated. Reach for `case` instead of either.
|
||||
|
||||
See sample: `case-true-of-for-long-condition-chains.bad.al`.
|
||||
|
||||
## See also
|
||||
|
||||
`boolean-operators-do-not-short-circuit.md` covers the underlying evaluation rule that makes the sequencing necessary in the first place.
|
||||
Loading…
Add table
Add a link
Reference in a new issue