mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-06 15:16:56 +01:00
47000475e3
Fixes two points from JesperSchulz's review on PR #136. 1. boolean-operators-do-not-short-circuit.md gave one fix — nested if — for and, or, and xor alike. That's only correct for and: nesting if A then if B then Action drops the A-true/B-false case of A or B, silently changing the result. Gives or its own early-exit pattern (if A then exit(true); exit(B)), warns explicitly against applying the and-rewrite to or, and clarifies that xor is not a short-circuit candidate in any language since its result always depends on both operands. Adds an or fixture (IsEligibleForFreeShipping) to both samples so an agent has a concrete pattern to match instead of extrapolating from the and-only examples. 2. case-true-of-for-long-condition-chains.md derived stop-at-first-match for one comma-separated value set from the documentation's guarantee about the first matching value set — plural, i.e. ordering across value sets, which is not the same claim. The good sample's Item.Get / Blocked pair depended on the one the docs don't make. Restructures the sample to only comma-group the three pure, order-independent checks; Get and Blocked keep their own value sets, in order, relying solely on the guarantee that is actually documented. Description and Anti Pattern now state that boundary so it isn't re-collapsed later. Also carries forward a parenthesis fix (not Item.Blocked in the collapsed bad sample) that was made two commits ago but never landed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
31 lines
1.4 KiB
AL
31 lines
1.4 KiB
AL
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;
|
|
|
|
procedure IsEligibleForFreeShipping(SalesHeader: Record "Sales Header"): Boolean
|
|
begin
|
|
// HasActiveLoyaltyBenefit runs even when the amount alone already qualifies,
|
|
// paying for the costly check on every evaluation instead of only the path
|
|
// where it can still change the outcome.
|
|
exit((SalesHeader."Amount Including VAT" >= 1000) or HasActiveLoyaltyBenefit(SalesHeader."Sell-to Customer No."));
|
|
end;
|
|
|
|
local procedure HasActiveLoyaltyBenefit(CustomerNo: Code[20]): Boolean
|
|
begin
|
|
exit(CustomerNo <> '');
|
|
end;
|
|
}
|