18 AL/BC patterns: style, data-modeling, web-services, appsource, breaking-changes, performance, testing (#156)

* Add 18 community AL/BC patterns across style, data-modeling, web-services, appsource, breaking-changes, performance, and testing

Contributed by CURABIS ApS, generalized from patterns observed across real AppSource/PTE development. Each article follows the knowledge file format (frontmatter, Description/Best Practice/Anti Pattern, sibling .good.al/.bad.al samples).

* Address Jesper Schulz-Wedde's review on PR #156

- Rename 3 articles so their .good.al/.bad.al companion stems match
  (do-not-change-primary-key, testfield-required-setup-field,
  al-identifiers-english), fixing the R14 orphan-sample errors.
- do-not-change-primary-key.good.al: include Flow in the new table's
  own primary key so it actually models the discriminating dimension.
- al-build-output-must-not-pollute-project-root.md: drop the
  unsubstantiated AL0197 causal claim and the non-existent
  al.outputPath setting; reframe as build-artifact hygiene sourced
  from ALTool --outfolder / al_build outputPath.
- prefer-email-module.md: Email Message is Codeunit 8904, not a table;
  distinguish it from the underlying Sent/Outbox/Draft storage.
- file-datatype-saas.md: File.Open/Create/Read/Write fails to compile
  against a Cloud-scoped project, it does not compile and silently
  fail at runtime.
- namespace-must-be-verified-from-source.md: narrow to "resolve from
  the referenced object's source or symbols," since source-file line
  one is not the only authoritative source (symbol packages, comments
  before the namespace line).
- test-data-must-be-random-and-complete.md: drop "assume an empty
  database" and "collision-free" absolutes; reframe around
  independence from unrelated business records and reserving explicit
  values for scenario-defining inputs.
- binary-choice-must-be-boolean.md: scope to genuine true/false
  semantics, not mechanical two-member-enum-to-boolean conversion.
- document-report-word-layout.md: scope down to a sourced Microsoft
  Learn recommendation instead of an unconditional performance
  guarantee; cite the three Learn pages.
- Wire the new articles into their review skills' candidate-selection
  signals (file-datatype-saas, prefer-email-module,
  namespace-must-be-verified-from-source, var-parameters-require-an-
  addressable-variable) so they can actually enter a worklist.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Fix dimension-management-wiring.md: ValidateShortcutDimCode and CreateDim
do not exist on the current DimensionManagement codeunit

Verified against microsoft/BCApps: the real master-table validation
procedure is ValidateDimValueCode (or ValidateShortcutDimValues when a
DimSetID is also needed), and the real document-side inheritance
procedure is GetDefaultDimID, not CreateDim. Caught from Jesper
Schulz-Wedde's review thread, which had been partially hidden by
GitHub's comment folding.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Address second round of Jesper Schulz-Wedde's review on PR #156

- dimension-management-wiring.md/.good.al: split into the two distinct
  models the article was conflating - master data (Default Dimension
  records via ValidateDimValueCode/SaveDefaultDim) vs. transactional/
  document data (a single Dimension Set ID assembled via AddDimSource +
  GetDefaultDimID, verified against BCApps' ExchRateAdjmtProcess.Codeunit.al).
  Added a compiling document-table example alongside the existing master
  table one.
- Deleted api-page-flowfields-must-be-calcfields (.md/.good.al/.bad.al):
  Microsoft's own FlowFields documentation states a FlowField used as a
  control's direct source expression is automatically calculated on any
  page - no API-page exception is documented, and none could be
  reproduced.
- prefer-email-module.bad.al/.md: Codeunit Mail has no Send/GetErrorDesc
  members; fixed to the real current 7-argument CreateMessage signature,
  and corrected the claim that the legacy path "still runs" - its base
  implementation no longer sends anything, only raises integration events.
- check-post-line-batch-pattern.md/.good.al: reframed from a universal
  invariant to the standard shape, naming the real Gen./Item/CA/Res./Job/
  Insurance/Mfg. Item/FA Jnl.-Check Line/-Post Line/-Post Batch codeunits
  it's based on. Added the missing Check Line companion codeunit so the
  good fixture is internally complete.
- test-data-must-be-random-and-complete.good.al: removed leftover
  "collision-free" wording contradicting the already-corrected article text.
- fixed-choice-set-must-use-enum-not-integer.md: removed the reintroduced
  state-count heuristic ("the line is the state count"), aligned with
  binary-choice-must-be-boolean.md's semantics-based distinction.
- namespace-must-be-verified-from-source.md: removed the false claim that
  the compiler and AL Language Server use different namespace-resolution
  rules.
- intrinsic-al-functions-must-use-modern-casing.md: removed the unverified
  claim that PascalCase is the VS Code formatter's default output.

Worklist completeness: added cues for the 8 rules in data-modeling,
testing, performance, and web-services that had none (Jesper's explicit
ask), plus the same gap in all 7 style rules from this PR (not explicitly
named this round, but the identical systemic issue) - 15 cues total across
al-data-modeling-review.md, al-testing-review.md, al-performance-review.md,
al-web-services-review.md, and al-style-review.md.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Fix remaining correctness issues from Jesper's 2026-09-15 re-review

- dimension-management-wiring: SaveDefaultDim's third argument is the
  shortcut dimension number (1-8), not the field's AL field ID; the
  fixture passed FieldNo(...) = 10. GetDefaultDimID's InheritFromDimSetID
  must be 0 when recomputing after the linking record changes, not the
  document's existing Dimension Set ID (which would retain the previous
  customer's leftover dimensions). Verified against
  DimensionManagement.Codeunit.al and BankDepositHeader.Table.al in the
  BCApps reference clone.
- check-post-line-batch-pattern: "Post Line writes exactly one line to
  the ledger" overclaimed - Gen. Jnl.-Post Line alone calls InsertGLEntry
  from a dozen call sites (balancing entry, VAT, currency rounding,
  deferrals) and can write several G/L Entries per journal line.
  Reworded to "posts exactly one journal line" and softened the
  "distinct, non-overlapping responsibilities" absolute.
- namespace-must-be-verified-from-source.bad.al: dropped the "resolves
  in a local build, fails in VS Code" comment (taught an inherent
  compiler/language-server disagreement that isn't real); reframed as
  stale/cached symbols, matching the prose fix already made.
- file-datatype-saas.good.al: replaced the deprecated 5-argument
  UploadIntoStream overload with the current 2-argument one, and
  actually staged through TempBlob as the article's own Best Practice
  instructs (the declared TempBlob variable was previously unused).
- test-data-must-be-random-and-complete: no longer treats a
  short-but-valid value as defective merely for being "underfilled" -
  AL field lengths are maxima, not minimums. Scoped to missing values
  or a scenario with an explicit length/format requirement (e.g. a
  truncation test). Updated the al-testing-review.md routing cue to
  match.
- stored-derived-fields-must-not-be-exposed-directly: stopped mandating
  source-field exposure as part of the core pattern: the good fixture
  exposed only one of the derived value's two inputs (Hours Used, not
  Budgeted Hours), making the claimed "so the consumer can verify it"
  impossible. Reframed as an optional, all-or-nothing addition and
  fixed the fixture to expose both inputs.

Rebased onto upstream/main to resolve conflicts in
al-breaking-changes-review.md, al-data-modeling-review.md,
al-performance-review.md, and al-style-review.md against merged PRs
#148 and #153; all sides' worklist tokens/cues retained.

* Fix remaining READ-convention sample links across this PR's 18 articles

The same plain-backtick "See sample: \`x.good.al\`." form fixed on
al-methods-limited-during-write-transactions (PR #161) turned up
repo-wide on 15 more of this PR's articles - Knowledge-Retrieval.ps1
requires the markdown-link form to associate a sample with its
article. All 16 fixed; the four local validators (frontmatter,
knowledge-index, knowledge-retrieval, review-fixtures, skill-index)
pass.

* Fix two merge-critical correctness issues from Jesper's 2026-09-22 review

- api-page-key-fields-must-be-editable-on-insert.good.al and
  stored-derived-fields-must-not-be-exposed-directly.good.al: both were
  writable API pages missing DelayedInsert = true, contradicting this
  repo's own api-page-delayedinsert-true rule - the canonical "good"
  samples were teaching code BCQuality itself flags.
- dimension-management-wiring.good.al: UpdateDimensionSetID exited
  early when Customer.Get failed, leaving the previous customer's
  shortcut dimension and Dimension Set ID in place - the same staleness
  bug the InheritFromDimSetID = 0 fix (from the prior review round) was
  meant to prevent, just triggered by a failed lookup instead of a
  successful one. Now clears the shortcut field and recomputes with an
  empty source list on a failed lookup too, so GetDefaultDimID
  correctly returns an empty Dimension Set ID instead of never running.

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Michael Dieringer 2026-09-29 12:48:17 +02:00 • committed by GitHub
parent 07e324ddbc
commit 0a8c9a8556
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
56 changed files with 1144 additions and 3 deletions

View file

@ -0,0 +1,24 @@
---
bc-version: [all]
domain: style
keywords: [build, output, alpackages, artifact-hygiene, outfolder, project-root]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Write AL Build Artifacts to an Intentional Output Location
> Contributions welcome — open a PR to refine or extend this article.
## Description
An AL project's compiled `.app` file can be written to the project root by default, and current tooling explicitly supports choosing a different destination instead — `ALTool`'s `--outfolder` option and the `al_build` agent tool's `outputPath` parameter both exist for this. The problem this rule addresses is not that root-level output is technically invalid; it is agents leaving generated `.app` files scattered through arbitrary source locations, or treating a compiled artefact as if it were part of the source tree (committing it, editing around it, referencing it as a dependency by hand).
## Best Practice
Write build artifacts to a deliberate, dedicated output location — configured via the build tool actually in use (e.g. `ALTool --outfolder`, or an explicit `outputPath` on the agent build tool) — and add that folder to `.gitignore`. Treat a compiled `.app` as a build artifact, never as a source file to commit or hand-edit around.
## Anti Pattern
Letting `.app` files accumulate in arbitrary or unversioned locations without a deliberate output path, or committing compiled artefacts into source control alongside the AL files that produced them.

View file

@ -0,0 +1,11 @@
codeunit 50100 "Sales Amount Calculator"
{
procedure BeregnTotalbeloeb(var Salgslinje: Record "Sales Line"): Decimal
var
Beloeb: Decimal;
begin
Salgslinje.CalcSums(Amount);
Beloeb := Salgslinje.Amount;
exit(Beloeb);
end;
}

View file

@ -0,0 +1,11 @@
codeunit 50100 "Sales Amount Calculator"
{
procedure CalculateTotalAmount(var SalesLine: Record "Sales Line"): Decimal
var
TotalAmount: Decimal;
begin
SalesLine.CalcSums(Amount);
TotalAmount := SalesLine.Amount;
exit(TotalAmount);
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: style
keywords: [identifiers, naming, english, captions, translation]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Write AL Identifiers in English Only
> Contributions welcome — open a PR to refine or extend this article.
## Description
All AL identifiers — variables, procedures, parameters, fields, object names, enum values, and label identifiers — must be written in English, regardless of the developer's native language. Translations are handled separately through captions, tooltips, and XLIFF files, never by writing Danish, German, or other non-English identifiers directly into AL source code. This is not a stylistic preference: the public `microsoft/BCApps` codebase, maintained by engineers of many nationalities across hundreds of thousands of lines of AL, uses exclusively English identifiers, with all localization handled via caption properties and XLIFF rather than by changing identifier names. On any multi-contributor codebase, non-English identifiers make the code unreadable to contributors who don't share that language.
## Best Practice
Write every identifier in English, and translate developer intent rather than transliterating it — when a requirement is described in another language, the resulting variable, procedure, and field names should still read as English. Captions and tooltips may carry target-language text in the source file, with locale translations managed through XLIFF.
See sample: [`al-identifiers-english.good.al`](al-identifiers-english.good.al).
## Anti Pattern
Using native-language identifiers such as a Danish variable or procedure name in AL source code, relying on the fact that the code still compiles and runs correctly. This makes the code unreadable to non-native-language contributors and mixes localization concerns into source that should stay language-neutral.
See sample: [`al-identifiers-english.bad.al`](al-identifiers-english.bad.al).

View file

@ -0,0 +1,22 @@
table 50100 "Sales Task"
{
fields
{
field(1; "No."; Code[20]) { }
field(10; Status; Option)
{
OptionMembers = Active,Blocked;
}
}
}
codeunit 50100 "Sales Task Check"
{
procedure IsOverdue(DueDate: Date): Integer
begin
// 0 = No, 1 = Yes
if DueDate < Today then
exit(1);
exit(0);
end;
}

View file

@ -0,0 +1,16 @@
table 50100 "Sales Task"
{
fields
{
field(1; "No."; Code[20]) { }
field(10; Blocked; Boolean) { }
}
}
codeunit 50100 "Sales Task Check"
{
procedure IsOverdue(DueDate: Date): Boolean
begin
exit(DueDate < Today);
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: style
keywords: [boolean, option, yes-no, magic-number, variable-typing, field-typing]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Binary yes/no choices must be typed as Boolean, not Option or Integer
> Contributions welcome — open a PR to refine or extend this article.
## Description
When a field or variable represents a genuine true/false state — yes/no, on/off, active/inactive, blocked/not blocked — it should be typed `Boolean`. Modeling that same predicate as an `Option`/`Enum` with two members, or as an `Integer` with two magic-number values, adds a layer of indirection a reader has to resolve before understanding the code. This is about semantics, not member count: a domain concept that currently has exactly two named alternatives — Inbound/Outbound, Debit/Credit, Buy/Sell — is not automatically a Boolean in disguise. An `Enum` can be the clearer model there, including when it needs to implement an interface, preserve an existing contract, or leave room for a future third value. The distinction is whether the domain is genuinely a stable predicate, not how many states it currently has.
## Best Practice
Type a field or variable as `Boolean` when the domain concept is inherently a true/false state. Do not replace a meaningful two-option domain model with a Boolean solely because it currently has two values.
See sample: [`binary-choice-must-be-boolean.good.al`](binary-choice-must-be-boolean.good.al).
## Anti Pattern
Modeling a yes/no choice as an `Option` with two members, or as an `Integer` with magic-number values, forces every caller to remember which value means what and leaves room for a meaningless third value.
See sample: [`binary-choice-must-be-boolean.bad.al`](binary-choice-must-be-boolean.bad.al).

View file

@ -0,0 +1,22 @@
table 50101 "Course"
{
fields
{
field(1; "No."; Code[20]) { }
// 1 = Beginner, 2..3 = Intermediate, 4+ = Advanced
field(10; Difficulty; Integer) { }
}
}
codeunit 50100 "Course Level Helper"
{
procedure LevelText(Difficulty: Integer): Text
begin
case Difficulty of
1:
exit('Beginner');
2, 3:
exit('Intermediate');
end;
end;
}

View file

@ -0,0 +1,17 @@
enum 50100 "Course Difficulty"
{
Extensible = true;
value(0; Beginner) { }
value(1; Intermediate) { }
value(2; Advanced) { }
}
table 50101 "Course"
{
fields
{
field(1; "No."; Code[20]) { }
field(10; Difficulty; Enum "Course Difficulty") { }
}
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: style
keywords: [enum, option, integer, magic-number, variable-typing, field-typing]
technologies: [al]
countries: [w1]
application-area: [all]
---
# A fixed set of named choices must use Enum, not a raw Integer
> Contributions welcome — open a PR to refine or extend this article.
## Description
When a variable or field represents a fixed set of named, mutually exclusive states — a difficulty level, a document type, a processing status — it should be typed as `Enum` (or `Option` when extending an object that still uses the legacy type). Representing that same state as a plain `Integer` and tracking the meaning of each value in a comment or in a developer's head is a magic-number anti-pattern: the compiler cannot catch an out-of-range value, and branches read as opaque numbers instead of names. This is about semantics, not member count, matching `binary-choice-must-be-boolean.md`'s own distinction: a domain concept that is genuinely a stable true/false predicate belongs in `Boolean` even if someone represents it as a two-value `Enum`, while a domain concept with exactly two current named states is not automatically a Boolean in disguise — it stays an `Enum` when the states are named alternatives rather than a yes/no flag, or when it needs to implement an interface, preserve an existing contract, or leave room for a future third value.
## Best Practice
Declare an `Enum` with named values and branch on the enum value, not a raw number.
See sample: [`fixed-choice-set-must-use-enum-not-integer.good.al`](fixed-choice-set-must-use-enum-not-integer.good.al).
## Anti Pattern
Using a plain `Integer` field with the meaning of each value tracked only in a comment pushes the documentation of the states into something the compiler cannot check and a future maintainer cannot rely on.
See sample: [`fixed-choice-set-must-use-enum-not-integer.bad.al`](fixed-choice-set-must-use-enum-not-integer.bad.al).

View file

@ -0,0 +1,12 @@
codeunit 50100 "Sales Task Notify"
{
procedure NotifyUpdate(Count: Integer; CustomerNo: Code[20])
begin
MESSAGE('%1 records updated.', Count);
IF NOT CONFIRM('Delete %1?', FALSE, CustomerNo) THEN
EXIT;
ERROR(STRSUBSTNO('%1 must not be blank.', CustomerNo));
end;
}

View file

@ -0,0 +1,12 @@
codeunit 50100 "Sales Task Notify"
{
procedure NotifyUpdate(Count: Integer; CustomerNo: Code[20])
begin
Message('%1 records updated.', Count);
if not Confirm('Delete %1?', false, CustomerNo) then
exit;
Error(StrSubstNo('%1 must not be blank.', CustomerNo));
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: style
keywords: [casing, intrinsic-function, built-in-function, message, error, confirm, strsubstno, legacy, c-al, pascalcase]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Intrinsic AL function calls use modern casing, not legacy ALL-CAPS
> Contributions welcome — open a PR to refine or extend this article.
## Description
AL is case-insensitive, so `MESSAGE(...)`, `ERROR(...)`, `CONFIRM(...)`, and `STRSUBSTNO(...)` compile and run identically to `Message(...)`, `Error(...)`, `Confirm(...)`, and `StrSubstNo(...)`. Modern AL — Microsoft's own current samples and current reference codebases — writes intrinsic/built-in function calls in the casing Microsoft assigns to the function's declared name, typically PascalCase. This is a codebase-convention claim, not a claim about what the VS Code formatter enforces: the formatter normalizes particular syntax but is not a mechanism for recasing every intrinsic function call, so do not cite formatter behavior as the reason to follow this convention. ALL-CAPS calls are a holdover from classic C/AL and signal code that has not been modernized, even though it compiles and runs correctly. Reserved keywords such as `if`, `begin`, and `for` are a separate, already-tooled concern; intrinsic function names are identifiers, not keywords, so that tooling does not catch ALL-CAPS intrinsic function calls.
## Best Practice
Call intrinsic functions in their modern, PascalCase form.
See sample: [`intrinsic-al-functions-must-use-modern-casing.good.al`](intrinsic-al-functions-must-use-modern-casing.good.al).
## Anti Pattern
ALL-CAPS intrinsic function calls trip no compiler error, but they are a reliable signal that a code block was copied from old C/AL material or outdated training content rather than written against current AL conventions.
See sample: [`intrinsic-al-functions-must-use-modern-casing.bad.al`](intrinsic-al-functions-must-use-modern-casing.bad.al).

View file

@ -0,0 +1,13 @@
namespace Contoso.Extensions;
using System.Performance; // guessed; never verified against the source file
codeunit 50100 "Tooling Extension"
{
procedure Run()
var
ToolingPage: Page "Some Tooling Page"; // guessed namespace; can still resolve against a stale or cached symbol package, then fail once checked against the object's current source or a freshly downloaded one
begin
ToolingPage.Run();
end;
}

View file

@ -0,0 +1,16 @@
// Verified by reading line 1 of the source file for "Some Tooling Page":
// namespace System.Tooling;
namespace Contoso.Extensions;
using System.Tooling;
codeunit 50100 "Tooling Extension"
{
procedure Run()
var
ToolingPage: Page "Some Tooling Page";
begin
ToolingPage.Run();
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: style
keywords: [namespace, verification, source-of-truth, using-statement]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Resolve a namespace from the referenced object's source or symbols, never by guessing
> Contributions welcome — open a PR to refine or extend this article.
## Description
Since Business Central 2024 release wave 1, Microsoft's own objects are organized under a deep `Microsoft.*` namespace tree that has been renamed and restructured repeatedly. When adding a `using` directive for an existing AL object (table, codeunit, page, enum, interface, etc.), guessing its namespace from the object's name, from an older codebase, or from general familiarity produces a statement that can look plausible and still resolve to the wrong object, or fail to resolve at all, once checked against the object's actual current namespace. The reliable sources are the object's own source file (its `namespace` declaration) or, for a dependency without accessible source, its AL symbol package — not the object's name or a remembered convention.
## Best Practice
When referencing an existing AL object, resolve its namespace from that object's actual source file or symbol definition — never infer or invent one from its name, functional area, or naming convention.
See sample: [`namespace-must-be-verified-from-source.good.al`](namespace-must-be-verified-from-source.good.al).
## Anti Pattern
Writing a `using` statement from memory, from an incomplete path, or from a plausible-looking guess. It can appear correct while actually resolving to the wrong object, or fail to resolve, once checked against stale or mismatched symbols, a different build configuration, or the object's actual current source — not because the compiler and the AL Language Server apply different namespace-resolution rules; they don't.
See sample: [`namespace-must-be-verified-from-source.bad.al`](namespace-must-be-verified-from-source.bad.al).

View file

@ -0,0 +1,15 @@
codeunit 50100 "Customer Lookup"
{
procedure GetCustomerName(CustomerNo: Code[20]; var Name: Text[100])
var
Customer: Record Customer;
begin
if Customer.Get(CustomerNo) then
Name := Customer.Name;
end;
procedure Sample()
begin
GetCustomerName('10000', 'placeholder'); // compile error: literal is not addressable
end;
}

View file

@ -0,0 +1,17 @@
codeunit 50100 "Customer Lookup"
{
procedure GetCustomerName(CustomerNo: Code[20]; var Name: Text[100])
var
Customer: Record Customer;
begin
if Customer.Get(CustomerNo) then
Name := Customer.Name;
end;
procedure Sample()
var
CustName: Text[100];
begin
GetCustomerName('10000', CustName);
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: style
keywords: [var-parameter, by-reference, pass-by-reference, literal, constant, compile-error, procedure-call]
technologies: [al]
countries: [w1]
application-area: [all]
---
# A `var` parameter must be called with a variable, never a literal or expression
> Contributions welcome — open a PR to refine or extend this article.
## Description
When a procedure declares a parameter with `var`, that parameter is passed by reference: the callee writes back into the caller's own memory location. This means the argument at the call site must be an actual variable, something with an address. A string literal, a numeric constant, or a computed expression has no address to write back to, so passing one to a `var` parameter fails to compile. Before writing a call, check the callee's signature for `var` on each parameter position being supplied a literal or expression — if present, a variable declared in the caller's own scope must be used instead.
## Best Practice
Declare a variable in the caller's scope and pass it to the `var` parameter.
See sample: [`var-parameters-require-an-addressable-variable.good.al`](var-parameters-require-an-addressable-variable.good.al).
## Anti Pattern
Passing a literal or a computed expression to a `var` parameter position fails to compile, because neither has an address the callee can write back to.
See sample: [`var-parameters-require-an-addressable-variable.bad.al`](var-parameters-require-an-addressable-variable.bad.al).