mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
Add community knowledge on AL boolean operators not short-circuiting
AL gives no short-circuit (lazy) evaluation guarantee for and/or/xor — neither the AL operators nor the boolean operators documentation defines a lazy evaluation order. LLMs trained on C#, JavaScript, or SQL assume the left operand guards the right, which produces conditions where a guard does not protect an unsafe subscript or a field read after a failed Get, and where an expensive operand is paid on every path. Adds community/knowledge/performance/boolean-operators-do-not-short-circuit.md with good/bad AL companions. The guidance prefers nested if when one operand depends on another, while keeping and/or legitimate for operands that are independently safe and cheap, so a reviewer does not flag harmless bound checks. This is the first article in a community performance domain; the Microsoft performance review leaf skill already sources candidates by domain across every enabled layer, so no skill change is needed to reach it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
9fab60153f
commit
1496e72b7b
3 changed files with 72 additions and 0 deletions
|
|
@ -0,0 +1,18 @@
|
||||||
|
codeunit 50541 "Perf Sample NoShortCircuit Bad"
|
||||||
|
{
|
||||||
|
procedure ExceedsThreshold(var Thresholds: array[10] of Decimal; Index: Integer; Amount: Decimal): Boolean
|
||||||
|
begin
|
||||||
|
// Thresholds[Index] is evaluated even when Index is 0, so the leading range
|
||||||
|
// check does not prevent the subscript from being read out of range.
|
||||||
|
exit((Index >= 1) and (Index <= ArrayLen(Thresholds)) and (Amount > Thresholds[Index]));
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure IsBlockedCustomer(CustomerNo: Code[20]): Boolean
|
||||||
|
var
|
||||||
|
Customer: Record Customer;
|
||||||
|
begin
|
||||||
|
// The Get runs even for an empty CustomerNo, and Blocked is read even when the
|
||||||
|
// Get failed, so the result is taken from a record that was never loaded.
|
||||||
|
exit((CustomerNo <> '') and Customer.Get(CustomerNo) and (Customer.Blocked <> Customer.Blocked::" "));
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,24 @@
|
||||||
|
codeunit 50540 "Perf Sample NoShortCircuit Good"
|
||||||
|
{
|
||||||
|
procedure ExceedsThreshold(var Thresholds: array[10] of Decimal; Index: Integer; Amount: Decimal): Boolean
|
||||||
|
begin
|
||||||
|
// 'and' is safe here: both operands are cheap and neither depends on the other.
|
||||||
|
if (Index >= 1) and (Index <= ArrayLen(Thresholds)) then
|
||||||
|
// The subscript lives in its own if, so it is never evaluated out of range.
|
||||||
|
if Amount > Thresholds[Index] then
|
||||||
|
exit(true);
|
||||||
|
exit(false);
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure IsBlockedCustomer(CustomerNo: Code[20]): Boolean
|
||||||
|
var
|
||||||
|
Customer: Record Customer;
|
||||||
|
begin
|
||||||
|
// The cheap test runs first, and the field is read only after Get succeeded.
|
||||||
|
if CustomerNo = '' then
|
||||||
|
exit(false);
|
||||||
|
if not Customer.Get(CustomerNo) then
|
||||||
|
exit(false);
|
||||||
|
exit(Customer.Blocked <> Customer.Blocked::" ");
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,30 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: performance
|
||||||
|
keywords: [short-circuit, lazy-evaluation, boolean-operators, nested-if, guard, and-operator, or-operator]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# AL boolean operators do not short-circuit
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
AL gives no short-circuit (lazy) evaluation guarantee for `and`, `or`, and `xor`: every operand of a boolean expression is evaluated, even when the leftmost operand already determines the result. Neither the AL operators documentation nor the boolean operators documentation defines a lazy evaluation order, so code must not depend on one. Developers arriving from C#, JavaScript, or SQL routinely assume the left operand guards the right; in AL it does not. The right operand still runs, so its cost is paid on every evaluation, and a check intended to protect an unsafe expression — an array subscript, a division, a field read that is only valid after a successful `Get` — does not protect it.
|
||||||
|
|
||||||
|
## 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.
|
||||||
|
|
||||||
|
See sample: `boolean-operators-do-not-short-circuit.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
A single condition that joins a guard with an operand depending on that guard, or with an expensive operand, using `and` or `or`. The consequence is either wasted work on every evaluation — a database call or validation procedure invoked even when the outcome is already decided — or a runtime error or silently wrong result that the guard was written to prevent. Detection signals: an operand that indexes an array or list with a variable whose bounds are checked in a sibling operand; `Record.Get(...)` or a `Find`/`IsEmpty` call as one operand of `and` with a field read of the same record as another; a boolean-returning procedure call combined with a cheap field test. The pattern is common in code ported from a language that does short-circuit, and in conditions grown by appending a clause to an existing `if`.
|
||||||
|
|
||||||
|
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.
|
||||||
Loading…
Add table
Add a link
Reference in a new issue