mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
c39f723cae
* 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> * 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> * Comma-group the pure value sets in the case false of sample Review feedback: the five value sets sharing exit(false) should be comma separated. Applied to the three that are pure field reads with no order dependency. The Get and the Blocked read keep their own value sets. The documentation guarantees that the first matching value set executes, which orders matching across separate value sets; it says nothing about evaluation within one comma-separated set, and the natural lowering of that is an equality-or chain — where AL's or does not short-circuit. Grouping the Get with the checks that must precede it would rest the sample's correctness on undocumented behaviour, which is the defect these two articles exist to prevent. Encodes the boundary in the Best Practice section so the grouping is applied where it is safe and not where it is not, and scopes the repeated-exit detection signal explicitly to nested chains so the case sample does not read as its own anti-pattern. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use a single exit(false) for the whole case false of chain Review decision: all five conditions share one action, so they share one comma-separated value set with a single exit(false). Aligns the article's Best Practice with the sample — it previously told authors to keep side-effecting conditions in their own value set, which the sample no longer does — and drops the now-contradictory wording about listing the failure action per condition. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Drop the parentheses from the case false of value sets Applies NKarolak's review suggestion: a case value set needs no parentheses around a comparison. Encodes the rationale in the Best Practice section, since it is a real advantage of the pattern and is not documented elsewhere in the repo. The AL operator hierarchy places and/xor above the comparison operators and or just above them too, so parentheses are mandatory in an and chain — A = B and C = D misparses without them — while a case value set has no and to bind tighter and needs none. That inverts the precedence most developers arrive with from C#. Also notes in the bad sample that its parentheses are not optional, so the two samples contrast on parentheses as well as on evaluation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Address correctness review: or/xor pattern, case value-set ordering 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> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|---|---|---|
| .. | ||
| avoid-currpage-update-in-onaftergetrecord.bad.al | ||
| avoid-currpage-update-in-onaftergetrecord.good.al | ||
| avoid-currpage-update-in-onaftergetrecord.md | ||
| batch-number-series-instead-of-getnextno-per-row.bad.al | ||
| batch-number-series-instead-of-getnextno-per-row.good.al | ||
| batch-number-series-instead-of-getnextno-per-row.md | ||
| boolean-operators-do-not-short-circuit.bad.al | ||
| boolean-operators-do-not-short-circuit.good.al | ||
| boolean-operators-do-not-short-circuit.md | ||
| case-true-of-for-long-condition-chains.bad.al | ||
| case-true-of-for-long-condition-chains.good.al | ||
| case-true-of-for-long-condition-chains.md | ||
| changecompany-in-loop-drops-caches.bad.al | ||
| changecompany-in-loop-drops-caches.good.al | ||
| changecompany-in-loop-drops-caches.md | ||
| dataaccessintent-readonly-on-analytical-objects.bad.al | ||
| dataaccessintent-readonly-on-analytical-objects.good.al | ||
| dataaccessintent-readonly-on-analytical-objects.md | ||
| guiallowed-guard-on-pages-used-as-odata.bad.al | ||
| guiallowed-guard-on-pages-used-as-odata.good.al | ||
| guiallowed-guard-on-pages-used-as-odata.md | ||
| httpclient-inside-write-transaction-holds-locks.bad.al | ||
| httpclient-inside-write-transaction-holds-locks.good.al | ||
| httpclient-inside-write-transaction-holds-locks.md | ||
| isempty-before-findset-is-extra-round-trip.bad.al | ||
| isempty-before-findset-is-extra-round-trip.good.al | ||
| isempty-before-findset-is-extra-round-trip.md | ||
| oncompanyopen-subscribers-must-not-do-io.bad.al | ||
| oncompanyopen-subscribers-must-not-do-io.good.al | ||
| oncompanyopen-subscribers-must-not-do-io.md | ||
| page-background-tasks-for-expensive-cues.bad.al | ||
| page-background-tasks-for-expensive-cues.good.al | ||
| page-background-tasks-for-expensive-cues.md | ||
| pass-var-record-to-preserve-partial-load-enumerator.bad.al | ||
| pass-var-record-to-preserve-partial-load-enumerator.good.al | ||
| pass-var-record-to-preserve-partial-load-enumerator.md | ||
| prefer-related-table-over-extension-on-hot-ledgers.bad.al | ||
| prefer-related-table-over-extension-on-hot-ledgers.good.al | ||
| prefer-related-table-over-extension-on-hot-ledgers.md | ||
| query-results-bypass-primary-key-cache.bad.al | ||
| query-results-bypass-primary-key-cache.good.al | ||
| query-results-bypass-primary-key-cache.md | ||
| reset-clears-partial-record-selection.bad.al | ||
| reset-clears-partial-record-selection.good.al | ||
| reset-clears-partial-record-selection.md | ||
| skip-setloadfields-on-write-and-transferfields.bad.al | ||
| skip-setloadfields-on-write-and-transferfields.good.al | ||
| skip-setloadfields-on-write-and-transferfields.md | ||
| use-dedicated-lookup-pages-not-full-lists.bad.al | ||
| use-dedicated-lookup-pages-not-full-lists.good.al | ||
| use-dedicated-lookup-pages-not-full-lists.md | ||
| validate-on-partial-record-forces-jit.bad.al | ||
| validate-on-partial-record-forces-jit.good.al | ||
| validate-on-partial-record-forces-jit.md | ||