mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 17:36:53 +01:00
Integrate current review domains
Merge current main because it added four active AL review leaves, then extend the findings domain contract across all 15 leaves, generic composition guidance, plugin documentation, and the self-review knowledge-upgrade path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77d0a40e-8bf5-40ac-a450-40eb0255db03
This commit is contained in:
Jesper Schulz-Wedde
commit
55aa40eb9e
269 changed files with 4806 additions and 827 deletions
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [27..]
|
||||
domain: appsource
|
||||
keywords: [app-json, help-url, copilot, grounding, documentation, url-depth, contexturl]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Keep the Copilot help URL to two path levels
|
||||
|
||||
## Description
|
||||
|
||||
The `help` URL declared in `app.json` is what Copilot uses to ground answers about your app. That URL may be at most **two path levels** deep (for example `https://contoso.com/docs/myapp`). If you point it at a deeper path (three or more segments), Copilot does not use the URL as given: it truncates to the first two levels, drops any fragments and query strings, and then grounds on **all** content beneath that two-level path. The failure is silent — there is no build error — and the practical effect is worse answers, because Copilot may ingest sibling apps' documentation that lives under the same two-level parent.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Organize per-app documentation so the canonical help page sits no deeper than two path levels, and confirm during testing that Copilot citations resolve to your app's content rather than a broader parent. If your docs naturally nest deeper, give each app a dedicated two-level path it owns.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Setting `help` to a deep, tidy-looking docs path such as `https://contoso.com/docs/products/erp/myapp/setup`. Copilot truncates it to `…/docs/products`, then grounds on everything under that node — pulling in unrelated content and degrading answer quality for your users.
|
||||
|
|
@ -0,0 +1,43 @@
|
|||
// Anti-pattern: an own object with no affix. Another app that also defines a
|
||||
// "Loyalty Tier" table cannot be installed alongside this one.
|
||||
table 50379 "Loyalty Tier"
|
||||
{
|
||||
Caption = 'Loyalty Tier';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Code"; Code[20])
|
||||
{
|
||||
Caption = 'Code';
|
||||
}
|
||||
field(10; Description; Text[100])
|
||||
{
|
||||
Caption = 'Description';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Code")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Anti-pattern (the common half-measure): the extension object carries the
|
||||
// affix, but the field it adds to the standard Customer table does not. That
|
||||
// unaffixed field still collides with any other app that adds "Loyalty Points"
|
||||
// to Customer, and AS0011 flags it.
|
||||
tableextension 50378 "ABC Customer Ext" extends Customer
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(50378; "Loyalty Points"; Integer)
|
||||
{
|
||||
Caption = 'Loyalty Points';
|
||||
DataClassification = CustomerContent;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,40 @@
|
|||
// Own object: the affix "ABC" is carried at object-name level.
|
||||
table 50377 "ABC Loyalty Tier"
|
||||
{
|
||||
Caption = 'Loyalty Tier';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Code"; Code[20])
|
||||
{
|
||||
Caption = 'Code';
|
||||
}
|
||||
field(10; Description; Text[100])
|
||||
{
|
||||
Caption = 'Description';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Code")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Extension of a standard object: the added field is individually affixed,
|
||||
// because the object name (Customer) belongs to the base application.
|
||||
tableextension 50376 "ABC Customer Ext" extends Customer
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(50376; "Loyalty Points ABC"; Integer)
|
||||
{
|
||||
Caption = 'Loyalty Points';
|
||||
DataClassification = CustomerContent;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: appsource
|
||||
keywords: [object-affix, prefix, suffix, as0011, appsourcecop, collision, tableextension]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Apply a reserved affix to objects and to members added to base objects
|
||||
|
||||
## Description
|
||||
|
||||
An AppSource extension must carry a reserved affix — a prefix or a suffix of at least three characters — on the names of the objects it owns **and** on any field, key, control, or action it adds to a base-application object. The affix is registered with Microsoft; when two coexisting extensions would otherwise collide, the registrant of the affix wins. Without it, two apps that both add a `Loyalty Points` field to `Customer`, or both define a `Loyalty Tier` table, cannot be installed side by side.
|
||||
|
||||
AppSourceCop enforces this. The primary rule is AS0011 ("An affix is required"); the affixes are configured through `mandatoryAffixes` (and `mandatoryPrefix`) in `AppSourceCop.json`. Two placements matter and are easy to get half-right: an object you define carries the affix at **object-name** level, while a member you add to a **standard** object carries the affix on that **member's** name. Adding an affixed object is not enough — an unaffixed field bolted onto `Customer` still collides and still fails validation.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Own objects are named with the affix (e.g. a table `ABC Loyalty Tier`), and every field or action added to a standard object is individually affixed (e.g. `Loyalty Points ABC` on a `Customer` tableextension).
|
||||
|
||||
See sample: `object-affixes-prevent-collisions.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Unaffixed object or member names, or the common half-measure: the extension object carries the affix but a field it adds to a standard table does not. AS0011 flags the missing affix and the field can still collide with another app.
|
||||
|
||||
See sample: `object-affixes-prevent-collisions.bad.al`.
|
||||
|
|
@ -1,7 +1,7 @@
|
|||
codeunit 50305 "Net Amount Api Good"
|
||||
{
|
||||
// Old name kept and marked obsolete: callers still compile but get a warning
|
||||
// pointing at the replacement, with a tag recording the removal target version.
|
||||
// Old name kept during the warning window. The tag records when obsoletion
|
||||
// began; a later release deletes the method after consumers have migrated.
|
||||
[Obsolete('Use CalculateNetAmount instead.', '25.0')]
|
||||
procedure CalcNet(GrossAmount: Decimal; TaxRate: Decimal): Decimal
|
||||
begin
|
||||
|
|
|
|||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
Deleting or renaming a published procedure (or object) in a single release is a hard break: dependent extensions that reference it stop compiling the moment they pick up the new version, with no warning window to migrate. AL provides a staged deprecation lifecycle precisely so consumers get advance notice. For a procedure, apply the `[Obsolete('reason', 'tag')]` attribute: the member keeps working but every caller gets a compiler warning naming the replacement and the target version. The member stays through a deprecation window — at least one major release — before it is finally removed. Object- and field-level members use the matching `ObsoleteState = Pending` → `Removed` property progression. LLMs trained to "clean up" code often delete or rename the old member immediately, skipping the window entirely.
|
||||
Deleting or renaming a published procedure (or object) in a single release is a hard break: dependent extensions that reference it stop compiling the moment they pick up the new version, with no warning window to migrate. AL provides staged deprecation so consumers get advance notice. A procedure uses `[Obsolete('reason', 'tag')]`: it remains callable but callers receive a compiler warning naming the replacement and the version in which obsoletion began. Methods do not have `ObsoleteState`; after the deprecation window, the method is deleted, commonly through versioned preprocessor cleanup. Objects and fields instead use the `ObsoleteState = Pending` to `Removed` property progression.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When a published procedure is superseded, keep it in place and mark it `[Obsolete('Use CalculateNetAmount instead.', '25.0')]`, where the message names the replacement and the tag records the target version for removal. Have the obsolete member forward to the new one so behavior is preserved during the window. Only after the deprecation window has elapsed — a later release — change its state to removed. This gives every dependent app a compile-time signal and time to migrate before anything actually disappears.
|
||||
When a published procedure is superseded, keep it in place and mark it `[Obsolete('Use CalculateNetAmount instead.', '25.0')]`, where the message names the replacement and the tag records when the method became obsolete. Have the obsolete member forward to the new one so behavior is preserved during the window. Only after the deprecation window has elapsed should a later release delete the method. For an object or field, use `Pending` during the warning window and `Removed` afterward.
|
||||
|
||||
See sample: `deprecate-public-members-with-the-obsolete-lifecycle.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Renaming or deleting the published `CalcNet` procedure in place — replacing it with `CalculateNetAmount` and nothing else — so consumers calling `CalcNet` break immediately with no deprecation notice. Detection: a previously shipped non-`local` procedure that vanished or was renamed between versions with no `[Obsolete]` marker left behind on a kept member. Mark it obsolete and keep it for a window instead.
|
||||
Renaming or deleting the published `CalcNet` procedure in place — replacing it with `CalculateNetAmount` and nothing else — so consumers calling `CalcNet` break immediately with no deprecation notice. Detection: a previously shipped non-`local` procedure that vanished or was renamed between versions with no `[Obsolete]` marker left behind during a prior warning window. Do not suggest `ObsoleteState = Removed` for a method; that property belongs to supported object and element types.
|
||||
|
||||
See sample: `deprecate-public-members-with-the-obsolete-lifecycle.bad.al`.
|
||||
|
|
|
|||
|
|
@ -1,10 +1,10 @@
|
|||
codeunit 50320 "Payment Client Good"
|
||||
{
|
||||
var
|
||||
AccessToken: Text;
|
||||
AccessToken: SecretText;
|
||||
|
||||
// Credential flows inward through an internal setter and never leaves the object.
|
||||
internal procedure SetAccessToken(NewToken: Text)
|
||||
// Credential remains SecretText as it flows inward and is stored.
|
||||
internal procedure SetAccessToken(NewToken: SecretText)
|
||||
begin
|
||||
AccessToken := NewToken;
|
||||
end;
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [23..]
|
||||
domain: breaking-changes
|
||||
keywords: [sensitive-data, secrettext, token, credential, public-api, access-boundary]
|
||||
technologies: [al]
|
||||
|
|
|
|||
|
|
@ -3,9 +3,9 @@ table 50311 "Customer Profile Bad"
|
|||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
// Breaking: the published "Email" field was renamed in place. Dependent
|
||||
// extensions that reference "Email" stop compiling, and the data stored in
|
||||
// the old column is orphaned on upgrade.
|
||||
// Breaking: the published field was renamed while retaining ID 2.
|
||||
// AppSourceCop AS0005 rejects the compatibility change; retaining the ID
|
||||
// does not by itself mean the stored column was dropped and re-created.
|
||||
field(2; "Contact Email"; Text[80]) { }
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,20 +7,20 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# Obsolete published table fields instead of deleting or renaming them
|
||||
# Obsolete published table fields instead of deleting or renumbering them
|
||||
|
||||
## Description
|
||||
|
||||
A table field that has shipped carries two contracts at once: extensions reference it by name, and the database holds data in its column. Deleting the field, or renaming it (which the platform treats as drop-plus-add), breaks dependent code at compile time and discards the stored data — a silent data-loss event on upgrade. The fix is the same staged lifecycle used for objects: set `ObsoleteState = Pending` together with `ObsoleteReason` and an `ObsoleteTag` naming the target version, ship the new field alongside, migrate data during the window, and only switch the old field to `ObsoleteState = Removed` in a later release once nothing depends on it. LLMs often "tidy" a schema by renaming a field in place, not realizing this is both a breaking change and a data-loss risk.
|
||||
A shipped table field carries both a source-level contract and persisted data. Renaming a field while retaining its ID is prohibited by AppSourceCop AS0005 and can break dependent extensions, but it is not inherently a drop-and-readd operation and should not be described as automatic data loss. Deleting the field or replacing it under a different ID is the data-loss risk: the old field storage is no longer represented unless data is migrated. The supported path is to keep the old field and obsolete it, add a replacement under a new ID, and migrate values before later removal.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Add the replacement field, then mark the old field `ObsoleteState = Pending` with an `ObsoleteReason` that names the replacement and an `ObsoleteTag` carrying the target version (for example `'25.0'`). Keep the obsolete field readable so an upgrade codeunit can copy its data into the new field during the deprecation window. Move it to `ObsoleteState = Removed` only in a later major version, after the window has passed and data has migrated.
|
||||
Add the replacement field under a new ID, then mark the old field `ObsoleteState = Pending` with an `ObsoleteReason` that names the replacement and an `ObsoleteTag` recording the obsoletion version. Keep the old field readable so an upgrade codeunit can copy its data during the deprecation window. Move it to `ObsoleteState = Removed` only in a later release, after the window has passed and data has migrated.
|
||||
|
||||
See sample: `obsolete-table-fields-instead-of-deleting-them.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Renaming the published `Email` field to `Contact Email` directly in the table — or deleting it — so dependent extensions that reference `Email` break and the column's stored values are orphaned on upgrade. Detection: a previously shipped field removed or renamed in a table or table extension with no `ObsoleteState = Pending` step preserving the original. Obsolete the field through the lifecycle instead.
|
||||
Renaming published `Email` to `Contact Email` with the same ID violates the compatibility contract and AS0005, even though the retained ID does not itself imply a fresh empty column. Deleting `Email` or moving the replacement to another ID without migration additionally risks losing its stored values. Detection: a previously shipped field removed, renumbered, or renamed with no retained `Pending` field and migration path.
|
||||
|
||||
See sample: `obsolete-table-fields-instead-of-deleting-them.bad.al`.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,71 @@
|
|||
table 50372 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
}
|
||||
field(10; Name; Text[100])
|
||||
{
|
||||
Caption = 'Name';
|
||||
}
|
||||
field(20; Blocked; Boolean)
|
||||
{
|
||||
Caption = 'Blocked';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
// Anti-pattern: the block check sits in the master's own trigger. Editing a
|
||||
// blocked member is rare; referencing it is constant, and references never
|
||||
// fire OnModify. So this stops nothing that matters.
|
||||
trigger OnModify()
|
||||
begin
|
||||
TestField(Blocked, false);
|
||||
end;
|
||||
}
|
||||
|
||||
table 50373 "Loyalty Point Entry"
|
||||
{
|
||||
Caption = 'Loyalty Point Entry';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
Caption = 'Entry No.';
|
||||
AutoIncrement = true;
|
||||
}
|
||||
field(10; "Member No."; Code[20])
|
||||
{
|
||||
Caption = 'Member No.';
|
||||
TableRelation = "Loyalty Member"."No.";
|
||||
// No block check on the referencing side: a line can freely
|
||||
// reference a blocked member, and posting proceeds unchecked.
|
||||
}
|
||||
field(20; Points; Integer)
|
||||
{
|
||||
Caption = 'Points';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,84 @@
|
|||
table 50370 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
}
|
||||
field(10; Name; Text[100])
|
||||
{
|
||||
Caption = 'Name';
|
||||
}
|
||||
// Blocked is inert data here: the master carries the flag but no logic.
|
||||
field(20; Blocked; Boolean)
|
||||
{
|
||||
Caption = 'Blocked';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
table 50371 "Loyalty Point Entry"
|
||||
{
|
||||
Caption = 'Loyalty Point Entry';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
Caption = 'Entry No.';
|
||||
AutoIncrement = true;
|
||||
}
|
||||
field(10; "Member No."; Code[20])
|
||||
{
|
||||
Caption = 'Member No.';
|
||||
TableRelation = "Loyalty Member"."No.";
|
||||
|
||||
trigger OnValidate()
|
||||
var
|
||||
LoyaltyMember: Record "Loyalty Member";
|
||||
begin
|
||||
if "Member No." = '' then
|
||||
exit;
|
||||
// Enforcement lives at the point of use: reject a blocked master
|
||||
// as soon as a line references it.
|
||||
LoyaltyMember.Get("Member No.");
|
||||
LoyaltyMember.TestField(Blocked, false);
|
||||
end;
|
||||
}
|
||||
field(20; Points; Integer)
|
||||
{
|
||||
Caption = 'Points';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
procedure Post()
|
||||
var
|
||||
LoyaltyMember: Record "Loyalty Member";
|
||||
begin
|
||||
// Re-check before committing the transaction, in case the member was
|
||||
// blocked after the line was created.
|
||||
LoyaltyMember.Get("Member No.");
|
||||
LoyaltyMember.TestField(Blocked, false);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: data-modeling
|
||||
keywords: [blocked-field, testfield, referencing-code, point-of-use, enforcement, journal-line]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Enforce `Blocked` where the master is used, not in the master itself
|
||||
|
||||
## Description
|
||||
|
||||
The `Blocked` field on a master record (`Item`, `Customer`, `Resource`, or a custom master) is inert data. The master table holds **no** logic that acts on it. Enforcement belongs in the **consuming** code: when a journal line, document line, or posting routine references the master by its `No.`, that referencing object tests the flag at the point of use, e.g. `LoyaltyMember.Get("Member No."); LoyaltyMember.TestField(Blocked, false);` in the line's `OnValidate` and again before posting.
|
||||
|
||||
Putting the block check inside the master's own `OnInsert`/`OnModify` does nothing to stop transactional use: a blocked master is edited rarely, but it is *referenced* constantly, and those references never touch the master's own triggers. Base BC follows this split — `Item.Blocked` is checked by sales/purchase/journal code, not by the `Item` table. A boolean `Blocked` uses `TestField(Blocked, false)`; an option-style block (e.g. `Sales`/`All`) needs the specific option compared at each relevant path.
|
||||
|
||||
## Best Practice
|
||||
|
||||
The referencing line validates `Master.TestField(Blocked, false)` in `OnValidate` of the reference field and re-checks before posting. The master table stays logic-free on `Blocked`.
|
||||
|
||||
See sample: `check-blocked-in-referencing-code-not-in-master.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
The block check sits in the master's own `OnModify`/`OnInsert` (so referencing and posting proceed unchecked), or there is no check at all on the referencing side.
|
||||
|
||||
See sample: `check-blocked-in-referencing-code-not-in-master.bad.al`.
|
||||
|
|
@ -0,0 +1,31 @@
|
|||
table 50361 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
// Anti-pattern: an autoincrement Integer surrogate used as the business key.
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
Caption = 'Entry No.';
|
||||
AutoIncrement = true;
|
||||
}
|
||||
field(10; Name; Text[100])
|
||||
{
|
||||
Caption = 'Name';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
// No OnInsert, no number series, no "No." code, and no "No. Series" field.
|
||||
// Records get an opaque integer users never see and cannot quote on the phone,
|
||||
// and the master is cut off from BC's standard numbering and manual-entry flow.
|
||||
}
|
||||
|
|
@ -0,0 +1,45 @@
|
|||
table 50360 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
NotBlank = true;
|
||||
}
|
||||
field(2; "No. Series"; Code[20])
|
||||
{
|
||||
Caption = 'No. Series';
|
||||
Editable = false;
|
||||
TableRelation = "No. Series";
|
||||
}
|
||||
field(10; Name; Text[100])
|
||||
{
|
||||
Caption = 'Name';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
trigger OnInsert()
|
||||
var
|
||||
LoyaltySetup: Record "Loyalty Setup";
|
||||
NoSeries: Codeunit "No. Series";
|
||||
begin
|
||||
if "No." = '' then begin
|
||||
LoyaltySetup.Get();
|
||||
LoyaltySetup.TestField("Member Nos.");
|
||||
"No. Series" := LoyaltySetup."Member Nos.";
|
||||
"No." := NoSeries.GetNextNo("No. Series");
|
||||
end;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: data-modeling
|
||||
keywords: [no-series, primary-key, code20, oninsert, autoincrement, number-assignment]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# A master table's `No.` primary key comes from a number series in `OnInsert`
|
||||
|
||||
## Description
|
||||
|
||||
In Business Central, a master table (Customer, Vendor, Item, and any custom equivalent) uses a single primary-key field named `No.` of type `Code[20]`. It is populated from a number series — configured on the feature's application-area setup table — inside the table's `OnInsert` trigger, but only when `No.` is still blank (so a user may still type a manual number when the series allows it). The record also keeps a non-editable `No. Series` `Code[20]` field recording which series produced the number.
|
||||
|
||||
This is not an `Integer` `AutoIncrement` key, a GUID, or the `SystemId`. Those are surrogate/system identifiers that users never see and cannot quote; BC's whole document flow — lookups, filtering, printed references, telephone support — depends on a short, human-readable, business-controlled `No.`. Use the modern assignment API described in `use-no-series-codeunit-not-noseriesmanagement.md`.
|
||||
|
||||
## Best Practice
|
||||
|
||||
`No.` `Code[20]` is the sole primary key; a non-editable `No. Series` `Code[20]` field records the source series. `OnInsert` checks `if "No." = ''`, reads the setup table, `TestField`s the configured series, stores it in `No. Series`, and assigns `No.` from the series.
|
||||
|
||||
See sample: `master-table-no-from-number-series-in-oninsert.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
An `Integer` `AutoIncrement` (or GUID / `SystemId`) primary key used as the business key, with no `OnInsert` number assignment. Records get an opaque identifier no user can reference, and the master no longer participates in the standard numbering and manual-entry behavior every other BC master follows.
|
||||
|
||||
See sample: `master-table-no-from-number-series-in-oninsert.bad.al`.
|
||||
|
|
@ -0,0 +1,39 @@
|
|||
table 50369 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
}
|
||||
field(10; Name; Text[100])
|
||||
{
|
||||
Caption = 'Name';
|
||||
}
|
||||
field(20; "Last Date Modified"; Date)
|
||||
{
|
||||
Caption = 'Last Date Modified';
|
||||
Editable = false;
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
trigger OnModify()
|
||||
begin
|
||||
"Last Date Modified" := Today();
|
||||
end;
|
||||
|
||||
// Missing OnRename: renaming the member changes the primary key without
|
||||
// firing OnModify, so "Last Date Modified" keeps its old, stale value and
|
||||
// change-detection logic downstream skips the renamed record.
|
||||
}
|
||||
|
|
@ -0,0 +1,40 @@
|
|||
table 50368 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
}
|
||||
field(10; Name; Text[100])
|
||||
{
|
||||
Caption = 'Name';
|
||||
}
|
||||
field(20; "Last Date Modified"; Date)
|
||||
{
|
||||
Caption = 'Last Date Modified';
|
||||
Editable = false;
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
trigger OnModify()
|
||||
begin
|
||||
"Last Date Modified" := Today();
|
||||
end;
|
||||
|
||||
trigger OnRename()
|
||||
begin
|
||||
"Last Date Modified" := Today();
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: data-modeling
|
||||
keywords: [last-date-modified, onmodify, onrename, audit-field, non-editable, stale-value]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Refresh `Last Date Modified` in both `OnModify` and `OnRename`
|
||||
|
||||
## Description
|
||||
|
||||
Master tables carry a non-editable `Last Date Modified` field of type `Date`. It records when the record last changed and is refreshed by table triggers, not by the user. The refresh must happen in **both** `OnModify` and `OnRename`.
|
||||
|
||||
The reason is a BC-specific trap: renaming a record changes its primary key and fires `OnRename` — it does **not** fire `OnModify`. A table that updates `Last Date Modified` only in `OnModify` therefore leaves a stale date behind every rename. Downstream logic that keys on this field (incremental sync, integration deltas, "changed since" reports) then silently skips the renamed record. Assign `Today` (the system date), not `WorkDate`, because the field reflects the real modification moment.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Both `OnModify` and `OnRename` set `"Last Date Modified" := Today();`, and the field is declared `Editable = false` so only the triggers maintain it.
|
||||
|
||||
See sample: `set-last-date-modified-in-onmodify-and-onrename.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Only `OnModify` assigns `Last Date Modified`. After a rename the value is stale, and any process that trusts it to detect changes misses the record.
|
||||
|
||||
See sample: `set-last-date-modified-in-onmodify-and-onrename.bad.al`.
|
||||
|
|
@ -0,0 +1,55 @@
|
|||
table 50366 "Loyalty Setup"
|
||||
{
|
||||
Caption = 'Loyalty Setup';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
// Anti-pattern: an autoincrement key lets the table hold many rows,
|
||||
// so "the setup" is no longer a single, well-known record.
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
Caption = 'Entry No.';
|
||||
AutoIncrement = true;
|
||||
}
|
||||
field(10; "Member Nos."; Code[20])
|
||||
{
|
||||
Caption = 'Member Nos.';
|
||||
TableRelation = "No. Series";
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
page 50367 "Loyalty Setup List"
|
||||
{
|
||||
// Anti-pattern: a List page over a setup table invites multiple rows and
|
||||
// never guarantees that a row exists to read.
|
||||
Caption = 'Loyalty Setup List';
|
||||
PageType = List;
|
||||
SourceTable = "Loyalty Setup";
|
||||
UsageCategory = Administration;
|
||||
ApplicationArea = All;
|
||||
|
||||
layout
|
||||
{
|
||||
area(Content)
|
||||
{
|
||||
repeater(Group)
|
||||
{
|
||||
field("Member Nos."; Rec."Member Nos.")
|
||||
{
|
||||
ApplicationArea = All;
|
||||
ToolTip = 'Specifies the number series used to assign member numbers.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,70 @@
|
|||
table 50364 "Loyalty Setup"
|
||||
{
|
||||
Caption = 'Loyalty Setup';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Primary Key"; Code[10])
|
||||
{
|
||||
Caption = 'Primary Key';
|
||||
}
|
||||
field(10; "Member Nos."; Code[20])
|
||||
{
|
||||
Caption = 'Member Nos.';
|
||||
TableRelation = "No. Series";
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Primary Key")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
procedure GetRecordOnce()
|
||||
begin
|
||||
if Rec.Get() then
|
||||
exit;
|
||||
Rec.Init();
|
||||
Rec.Insert();
|
||||
end;
|
||||
}
|
||||
|
||||
page 50365 "Loyalty Setup"
|
||||
{
|
||||
Caption = 'Loyalty Setup';
|
||||
PageType = Card;
|
||||
SourceTable = "Loyalty Setup";
|
||||
UsageCategory = Administration;
|
||||
ApplicationArea = All;
|
||||
InsertAllowed = false;
|
||||
DeleteAllowed = false;
|
||||
|
||||
layout
|
||||
{
|
||||
area(Content)
|
||||
{
|
||||
group(Numbering)
|
||||
{
|
||||
Caption = 'Numbering';
|
||||
field("Member Nos."; Rec."Member Nos.")
|
||||
{
|
||||
ApplicationArea = All;
|
||||
ToolTip = 'Specifies the number series used to assign member numbers.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
trigger OnOpenPage()
|
||||
begin
|
||||
Rec.Reset();
|
||||
if not Rec.Get() then begin
|
||||
Rec.Init();
|
||||
Rec.Insert();
|
||||
end;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: data-modeling
|
||||
keywords: [setup-table, insertallowed, deleteallowed, getrecordonce, primary-key, card-page]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# A setup table is a singleton: one blank-keyed row, no insert or delete
|
||||
|
||||
## Description
|
||||
|
||||
An application-area setup table (`Sales & Receivables Setup`, `Inventory Setup`, and any custom `* Setup`) holds exactly one record per company. Its primary key is a single `Code[10]` field named `Primary Key`, and the row's value is left blank. Nothing else identifies the row — there is only ever one.
|
||||
|
||||
The setup **card** page enforces the singleton: `InsertAllowed = false` and `DeleteAllowed = false` stop a second row or an empty table, and the page guarantees the row exists on first open — typically `OnOpenPage` with `if not Rec.Get() then begin Rec.Init(); Rec.Insert(); end;`, or a `GetRecordOnce` helper on the table. Consuming code then reads it with a plain `Get()`. The read side needs no access optimization — see `singleton-setup-tables-need-no-access-optimization.md`.
|
||||
|
||||
## Best Practice
|
||||
|
||||
`Primary Key` `Code[10]` is the sole key; the setup is surfaced through a Card page with `InsertAllowed = false`, `DeleteAllowed = false`, and an open-time guard that inserts the blank row if it is missing.
|
||||
|
||||
See sample: `setup-table-is-a-singleton.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
An `Integer` / `AutoIncrement` key, a page that allows insert or delete, or a List page over the setup table. Any of these lets the table hold zero or many rows, so "the setup" becomes ambiguous and `Get()` may fail or read the wrong record.
|
||||
|
||||
See sample: `setup-table-is-a-singleton.bad.al`.
|
||||
|
|
@ -0,0 +1,51 @@
|
|||
table 50363 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
|
||||
trigger OnValidate()
|
||||
begin
|
||||
if "No." = xRec."No." then
|
||||
exit;
|
||||
LoyaltySetup.Get();
|
||||
// Obsolete-pending: NoSeriesManagement.TestManual raises a
|
||||
// deprecation warning and is scheduled for removal.
|
||||
NoSeriesMgt.TestManual(LoyaltySetup."Member Nos.");
|
||||
"No. Series" := '';
|
||||
end;
|
||||
}
|
||||
field(2; "No. Series"; Code[20])
|
||||
{
|
||||
Caption = 'No. Series';
|
||||
Editable = false;
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
var
|
||||
LoyaltySetup: Record "Loyalty Setup";
|
||||
NoSeriesMgt: Codeunit NoSeriesManagement;
|
||||
|
||||
trigger OnInsert()
|
||||
begin
|
||||
if "No." = '' then begin
|
||||
LoyaltySetup.Get();
|
||||
LoyaltySetup.TestField("Member Nos.");
|
||||
// Obsolete-pending legacy assignment call; use codeunit "No. Series".
|
||||
NoSeriesMgt.InitSeries(LoyaltySetup."Member Nos.", xRec."No. Series", 0D, "No.", "No. Series");
|
||||
end;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,55 @@
|
|||
table 50362 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
|
||||
trigger OnValidate()
|
||||
var
|
||||
NoSeries: Codeunit "No. Series";
|
||||
begin
|
||||
if "No." = xRec."No." then
|
||||
exit;
|
||||
LoyaltySetup.Get();
|
||||
if not NoSeries.IsManual(LoyaltySetup."Member Nos.") then
|
||||
Error(ManualNosNotAllowedErr);
|
||||
"No. Series" := '';
|
||||
end;
|
||||
}
|
||||
field(2; "No. Series"; Code[20])
|
||||
{
|
||||
Caption = 'No. Series';
|
||||
Editable = false;
|
||||
TableRelation = "No. Series";
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
var
|
||||
LoyaltySetup: Record "Loyalty Setup";
|
||||
ManualNosNotAllowedErr: Label 'Numbers are assigned automatically. Allow manual numbers on the No. Series to enter one by hand.';
|
||||
|
||||
trigger OnInsert()
|
||||
var
|
||||
NoSeries: Codeunit "No. Series";
|
||||
begin
|
||||
if "No." = '' then begin
|
||||
LoyaltySetup.Get();
|
||||
LoyaltySetup.TestField("Member Nos.");
|
||||
"No. Series" := LoyaltySetup."Member Nos.";
|
||||
"No." := NoSeries.GetNextNo("No. Series");
|
||||
end;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [22..]
|
||||
domain: data-modeling
|
||||
keywords: [no-series, getnextno, ismanual, noseriesmanagement, obsolete-pending, testmanual]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Assign numbers with codeunit `"No. Series"`, not the obsolete `NoSeriesManagement`
|
||||
|
||||
## Description
|
||||
|
||||
Since 2023 release wave 1 (v22) the number-series API is codeunit **310** `"No. Series"`, called by name in AL. Its methods include `GetNextNo`, `PeekNextNo`, `IsManual`, `TestManual`, and `LookupRelatedNoSeries`. The older codeunit **396** `NoSeriesManagement` and its `InitSeries` / `SelectSeries` / `SetSeries` / `TestManual` methods are marked obsolete-pending: they still compile but raise a deprecation warning and are scheduled for removal, so they must not appear in new code.
|
||||
|
||||
LLMs reproduce the legacy `NoSeriesManagement` pattern because it dominates pre-2023 training data. Prefer the new codeunit: it has a cleaner surface and is the only version that survives the deprecation. (The numbers matter — `310` is the current codeunit; `396` is the legacy one being retired.) Verify signatures on learn.microsoft.com or in the `microsoft/BCApps` source before use.
|
||||
|
||||
## Best Practice
|
||||
|
||||
`OnInsert` assigns the number with `NoSeries.GetNextNo("No. Series")` where `NoSeries` is `Codeunit "No. Series"`. The `No.` field's `OnValidate` guards manual entry by calling `NoSeries.IsManual(...)` (or `TestManual`) before clearing `No. Series`.
|
||||
|
||||
See sample: `use-no-series-codeunit-not-noseriesmanagement.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`NoSeriesMgt.InitSeries(...)` for assignment and `NoSeriesMgt.TestManual(...)` for the manual check, where `NoSeriesMgt` is `Codeunit NoSeriesManagement`. Both are obsolete-pending and emit compiler warnings.
|
||||
|
||||
See sample: `use-no-series-codeunit-not-noseriesmanagement.bad.al`.
|
||||
|
|
@ -15,10 +15,13 @@ codeunit 50185 "Collect Errors Good Sample"
|
|||
until Item.Next() = 0;
|
||||
|
||||
if HasCollectedErrors() then begin
|
||||
CollectedErrors := GetCollectedErrors();
|
||||
// The default is false; true retrieves and clears the collection.
|
||||
CollectedErrors := GetCollectedErrors(true);
|
||||
// This blocking aggregate intentionally retains messages only.
|
||||
foreach CollectedError in CollectedErrors do
|
||||
ErrorText += CollectedError.Message() + '\';
|
||||
Message('The following must be fixed before posting:\%1', ErrorText);
|
||||
Error(ErrorInfo.Create(
|
||||
StrSubstNo('The following must be fixed before posting:\%1', ErrorText), false));
|
||||
end;
|
||||
end;
|
||||
}
|
||||
|
|
@ -30,8 +33,10 @@ codeunit 50186 "Collect Errors Item Check"
|
|||
trigger OnRun()
|
||||
begin
|
||||
if Rec.Description = '' then
|
||||
Error('Item %1 has no description.', Rec."No.");
|
||||
Error(ErrorInfo.Create(
|
||||
StrSubstNo('Item %1 has no description.', Rec."No."), true));
|
||||
if Rec."Unit Cost" <= 0 then
|
||||
Error('Item %1 must have a positive unit cost.', Rec."No.");
|
||||
Error(ErrorInfo.Create(
|
||||
StrSubstNo('Item %1 must have a positive unit cost.', Rec."No."), true));
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [19..]
|
||||
domain: error-handling
|
||||
keywords: [collectible-errors, errorbehavior, collect, getcollectederrors, hascollectederrors, validation, batch]
|
||||
technologies: [al]
|
||||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
By default a procedure stops on the first `Error`, so a user fixing ten bad rows must rerun the operation ten times. The collectible-errors feature postpones error handling to the end of the call: a procedure attributed `[ErrorBehavior(ErrorBehavior::Collect)]` keeps running as errors occur and gathers them, so all failures can be presented together. The collected errors are read with `HasCollectedErrors()` and `GetCollectedErrors()` (which returns a `List of [ErrorInfo]`); `ClearCollectedErrors()` empties the buffer. This is a platform mechanism most LLMs are unaware of — they reach for a manually concatenated `Text` buffer or a temporary error table instead.
|
||||
By default a procedure stops on the first `Error`, so a user fixing ten bad rows must rerun the operation ten times. The collectible-errors feature postpones error handling to the end of the call: a procedure attributed `[ErrorBehavior(ErrorBehavior::Collect)]` keeps running as collectible errors occur and gathers them, so all failures can be presented together. `GetCollectedErrors()` returns a `List of [ErrorInfo]` for the handler to inspect, but does not clear the collection by default; pass `true` to retrieve and clear in one call, or call `ClearCollectedErrors()` explicitly after retrieving. A handler can copy record information into a custom error page as Microsoft Learn demonstrates, or deliberately format only the messages into a final blocking error as this article's sample does.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Mark the orchestrating procedure `[ErrorBehavior(ErrorBehavior::Collect)]` and run each item's validation so one failure doesn't abandon the rest — typically by calling the per-item routine through `Codeunit.Run`. When the run finishes, inspect `HasCollectedErrors()` and surface `GetCollectedErrors()` to the user as a single list. Always handle the collected errors yourself: the platform's own guidance is that any errors still in the collected list when the procedure ends are concatenated into one dialog, which is hard for users to read.
|
||||
Mark the orchestrating procedure `[ErrorBehavior(ErrorBehavior::Collect)]` and run each item's validation so one failure doesn't abandon the rest — typically by calling the per-item routine through `Codeunit.Run`. When the run finishes, inspect `HasCollectedErrors()`, retrieve and clear the list with `GetCollectedErrors(true)`, and fail the operation with the collected messages. The sample intentionally produces a text aggregate and does not claim to retain record/field metadata in the final error. If that metadata is needed, map each `ErrorInfo` to a custom error UI before clearing, following the Microsoft Learn pattern. Do not replace validation failure with `Message`: clearing collected errors suppresses the platform failure, so the custom handler must still block the invalid operation.
|
||||
|
||||
See sample: `collect-validation-errors-with-errorbehavior.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Two shapes signal trouble. The first is hand-rolled accumulation — appending messages to a `Text` variable and showing them at the end — which reimplements the platform feature, loses each error's `ErrorInfo` structure, and skips telemetry classification. The second is applying `[ErrorBehavior(ErrorBehavior::Collect)]` but never calling `HasCollectedErrors`/`GetCollectedErrors`, so every collected error spills into the platform's concatenated end-of-procedure dialog. Detection: a `Collect` attribute with no matching `GetCollectedErrors` call, or a per-row loop that builds an error string by concatenation.
|
||||
Three shapes signal trouble. Hand-rolled accumulation reimplements collection and prevents the handler from receiving individual `ErrorInfo` values. A `Collect` procedure that never handles the collection falls back to the concatenated platform dialog. Finally, code that calls parameterless `GetCollectedErrors()`, assumes it cleared the list, and only shows a `Message` can both leave the errors collected and allow invalid processing to continue.
|
||||
|
||||
See sample: `collect-validation-errors-with-errorbehavior.bad.al`.
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@ codeunit 50190 "Error Type Good Sample"
|
|||
if not BucketInitialized(BucketId) then begin
|
||||
InternalErr.ErrorType := ErrorType::Internal;
|
||||
InternalErr.Message := StrSubstNo('Ledger bucket %1 was not initialized before posting.', BucketId);
|
||||
InternalErr.DetailedMessage := 'Internal invariant violated. Inspect the call stack captured in telemetry.';
|
||||
Error(InternalErr);
|
||||
end;
|
||||
end;
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [14..]
|
||||
domain: error-handling
|
||||
keywords: [errorinfo, errortype, internal, client, telemetry, diagnostics, generic-message]
|
||||
technologies: [al]
|
||||
|
|
@ -15,7 +15,7 @@ application-area: [all]
|
|||
|
||||
## Best Practice
|
||||
|
||||
Reserve `ErrorType::Internal` for errors the user cannot act on: corrupted internal state, an unreachable branch, a contract a caller violated. Set a precise, detail-rich `Message` and `DetailedMessage` for telemetry, raise it via `Error(ErrorInfo)`, and let the platform show the user a generic dialog. Keep `ErrorType::Client` (or a plain `Error`) for failures the user is expected to read and resolve — validation messages, missing setup, business-rule violations. The test is simple: if the message only makes sense to a developer, mark it `Internal`.
|
||||
Reserve `ErrorType::Internal` for errors the user cannot act on: corrupted internal state, an unreachable branch, a contract a caller violated. Set a precise, detail-rich `Message` for telemetry, raise it via `Error(ErrorInfo)`, and let the platform show the user a generic dialog. Keep `ErrorType::Client` (or a plain `Error`) for failures the user is expected to read and resolve — validation messages, missing setup, business-rule violations. The test is simple: if the message only makes sense to a developer, mark it `Internal`.
|
||||
|
||||
See sample: `errortype-internal-vs-client-for-diagnostics.good.al`.
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,23 @@
|
|||
table 50120 "FieldError Default Bad"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(2; "Discount %"; Decimal) { }
|
||||
field(3; "Currency Code"; Code[10]) { }
|
||||
}
|
||||
|
||||
procedure ValidateForRelease()
|
||||
begin
|
||||
// This re-tests a field and gives FieldError a fully formed sentence.
|
||||
// The framework already prepends the caption and appends the value,
|
||||
// so this renders as "Currency Code The Currency Code field must have
|
||||
// a value. in ..." — caption repeated, capital letter mid-sentence,
|
||||
// stray trailing clause.
|
||||
if "Currency Code" = '' then
|
||||
FieldError("Currency Code", 'The Currency Code field must have a value.');
|
||||
|
||||
if "Discount %" > 100 then
|
||||
FieldError("Discount %", 'The Discount % must not be greater than 100 percent.');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,21 @@
|
|||
table 50120 "FieldError Default Good"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(2; "Discount %"; Decimal) { }
|
||||
field(3; "Currency Code"; Code[10]) { }
|
||||
}
|
||||
|
||||
procedure ValidateForRelease()
|
||||
begin
|
||||
// TestField checks this required-field condition and raises the error
|
||||
// with caption and record context supplied by the framework.
|
||||
TestField("Currency Code");
|
||||
|
||||
// Condition already evaluated: pass only a lowercase predicate so it
|
||||
// reads as one sentence after the auto-inserted caption and value.
|
||||
if "Discount %" > 100 then
|
||||
FieldError("Discount %", 'cannot exceed 100');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,18 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: error-handling
|
||||
keywords: [fielderror, testfield, error-message, field-caption, lowercase-convention, record-context, validation]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# Rely On FieldError's Auto-Generated Context And Pass Only A Lowercase Predicate
|
||||
|
||||
## Description
|
||||
`Rec.FieldError(FieldNo)` does not just print the text you give it. Business Central automatically prepends the field caption, appends the current field value (when non-blank), and suffixes the table name and primary-key values for record identification. The optional second argument is only the middle predicate of that sentence — e.g. `"must be unique"`, not a whole self-contained message. Misunderstanding this leads to messages that duplicate the caption and value or read as broken grammar, because the framework's surrounding text is built to join a lowercase fragment.
|
||||
|
||||
## Best Practice
|
||||
For a plain required-field check, prefer `TestField`, which tests the condition and raises the error in one call. When the condition is non-trivial and has already been evaluated, call `FieldError(FieldNo)` with no message to get the localized default (`must have a value`, `is not valid`, etc.), or pass a short lowercase predicate such as `FieldError(FieldNo, 'must be a positive number')`. Start the custom text with a lowercase letter so it reads as one sentence with the auto-inserted caption, and use a field-number reference (or the field token) rather than a hard-coded field name so captions and translations stay correct. Let the framework supply the caption, value, table, and key context for you.
|
||||
|
||||
## Anti Pattern
|
||||
Re-testing a condition you already evaluated, or passing a fully formed sentence like `'The Amount field must be positive.'` to `FieldError`. The result reads as `Amount The Amount field must be positive. in Gen. Journal Line ...` — capital letter mid-sentence, caption and value repeated, and a stray trailing clause. Reviewer signals: a `FieldError` argument that names the field, restates the current value, starts with a capital letter, or ends with a period. Each is a sign the author treated `FieldError` like `Error` instead of as a predicate slotted into framework-generated context.
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
table 50122 "FieldError vs TestField Bad"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(2; "Posting Date"; Date) { }
|
||||
field(3; "Amount"; Decimal) { }
|
||||
}
|
||||
|
||||
procedure PostDocument()
|
||||
begin
|
||||
// FieldError performs no comparison and raises as soon as it is
|
||||
// reached, so this "check" terminates PostDocument every time — the
|
||||
// Posting Date is never actually tested, and the amount rule below is
|
||||
// dead code.
|
||||
FieldError("Posting Date", 'must be filled in');
|
||||
|
||||
if IsAmountOutsideAllowedRange("Amount") then
|
||||
Error('Amount is out of range.');
|
||||
end;
|
||||
|
||||
local procedure IsAmountOutsideAllowedRange(Value: Decimal): Boolean
|
||||
begin
|
||||
exit((Value < 0) or (Value > 1000000));
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,27 @@
|
|||
table 50122 "FieldError vs TestField Good"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(2; "Posting Date"; Date) { }
|
||||
field(3; "Amount"; Decimal) { }
|
||||
}
|
||||
|
||||
procedure PostDocument()
|
||||
begin
|
||||
// TestField performs this simple presence check and raises only when
|
||||
// the field is empty. Self-documenting prerequisite.
|
||||
TestField("Posting Date");
|
||||
|
||||
// Business logic has already determined the value is invalid;
|
||||
// FieldError raises a tailored, record-aware message with no
|
||||
// condition of its own.
|
||||
if IsAmountOutsideAllowedRange("Amount") then
|
||||
FieldError("Amount", 'is outside the approved posting range');
|
||||
end;
|
||||
|
||||
local procedure IsAmountOutsideAllowedRange(Value: Decimal): Boolean
|
||||
begin
|
||||
exit((Value < 0) or (Value > 1000000));
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,18 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: error-handling
|
||||
keywords: [fielderror, testfield, field-validation, onvalidate, error-message, mandatory-field, record-context]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# Choose `TestField` For Conditional Checks And `FieldError` For Already-Failed Validation
|
||||
|
||||
## Description
|
||||
`TestField` and `FieldError` look interchangeable but behave differently, and choosing the wrong one produces either dead code or a check that never fires. `TestField` performs the comparison itself and throws only when the field is empty or does not match the supplied value; `FieldError` performs no comparison and always raises an error the moment it is reached. Both attach the field caption and the record's primary-key context to the message automatically, which is why neither should be replaced by a hand-built `Error` call that interpolates the field name as a literal.
|
||||
|
||||
## Best Practice
|
||||
Use `TestField` when the condition is a simple presence-or-equality check on a single field — mandatory-field gates and prerequisite checks at the top of a procedure read clearly and self-document intent. Use `FieldError` inside an `OnValidate` trigger or a validation procedure where surrounding business logic has already determined the value is invalid and you want a specific, custom message. Rely on the built-in field-and-record context both methods add rather than re-stating the field name in the text.
|
||||
|
||||
## Anti Pattern
|
||||
Calling `FieldError` to "test" a field — placing it on a path that is reached unconditionally and expecting it to validate — terminates execution every time because `FieldError` never evaluates a condition. The inverse smell is reaching for `TestField` when the rule needs a tailored message, then bolting a vague generic string onto a check that cannot express the real business reason. A reviewer can spot the first by a `FieldError` that is not guarded by a preceding `if`, and the second by a `TestField` whose intent comment describes a condition more complex than presence or equality.
|
||||
|
|
@ -0,0 +1,17 @@
|
|||
codeunit 50301 "Try Return Bad"
|
||||
{
|
||||
procedure ImportDocument()
|
||||
begin
|
||||
// Ignoring the Boolean result makes this an ordinary, throwing call.
|
||||
TryImportDocument();
|
||||
end;
|
||||
|
||||
[TryFunction]
|
||||
local procedure TryImportDocument()
|
||||
begin
|
||||
Error(SourceRejectedErr);
|
||||
end;
|
||||
|
||||
var
|
||||
SourceRejectedErr: Label 'The source document was rejected.';
|
||||
}
|
||||
|
|
@ -0,0 +1,18 @@
|
|||
codeunit 50300 "Try Return Good"
|
||||
{
|
||||
procedure ImportDocument()
|
||||
begin
|
||||
if not TryImportDocument() then
|
||||
Error(ImportFailedErr);
|
||||
end;
|
||||
|
||||
[TryFunction]
|
||||
local procedure TryImportDocument()
|
||||
begin
|
||||
Error(SourceRejectedErr);
|
||||
end;
|
||||
|
||||
var
|
||||
ImportFailedErr: Label 'The document could not be imported.';
|
||||
SourceRejectedErr: Label 'The source document was rejected.';
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
bc-version: [13..]
|
||||
domain: error-handling
|
||||
keywords: [tryfunction, try-method, boolean-return, ignored-return-value, error-propagation]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Consume a TryFunction return value to enable try semantics
|
||||
|
||||
## Description
|
||||
|
||||
A procedure marked `[TryFunction]` catches errors only when the caller uses its Boolean return value. An assignment or conditional makes the invocation a try-method call; a bare call is treated as an ordinary procedure call and exposes errors as usual. The attribute alone does not make every invocation non-throwing.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Consume the result directly: assign it to a Boolean or use the call in an `if` condition. Handle `false` immediately while the last-error state still describes that failure.
|
||||
|
||||
See sample: `ignored-tryfunction-return-disables-try-semantics.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling a `[TryFunction]` procedure as a standalone statement and assuming the attribute suppresses its errors. The call has ordinary error semantics because its Boolean result is ignored.
|
||||
|
||||
See sample: `ignored-tryfunction-return-disables-try-semantics.bad.al`.
|
||||
|
|
@ -1,21 +1,14 @@
|
|||
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||
// Demonstration-only AL. Version 1 exposed PostDocument(SalesHeader).
|
||||
codeunit 50251 "Param Append Bad Sample"
|
||||
{
|
||||
procedure PostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
||||
var
|
||||
IsHandled: Boolean;
|
||||
procedure PostDocument(var SalesHeader: Record "Sales Header")
|
||||
begin
|
||||
IsHandled := false;
|
||||
// Anti-pattern: 'CalledFromBatch' was inserted before the existing
|
||||
// IsHandled parameter, shifting it and breaking the argument positions
|
||||
// every existing subscriber relied on.
|
||||
OnBeforePostDocument(SalesHeader, CalledFromBatch, IsHandled);
|
||||
if IsHandled then
|
||||
exit;
|
||||
// Existing callers cannot supply the newly required argument.
|
||||
OnBeforePostDocument(SalesHeader);
|
||||
end;
|
||||
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean; var IsHandled: Boolean)
|
||||
procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||
// Demonstration-only AL. Version 1 had SalesHeader and IsHandled parameters.
|
||||
codeunit 50250 "Param Append Good Sample"
|
||||
{
|
||||
procedure PostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
||||
|
|
@ -6,15 +6,24 @@ codeunit 50250 "Param Append Good Sample"
|
|||
IsHandled: Boolean;
|
||||
begin
|
||||
IsHandled := false;
|
||||
// The new 'CalledFromBatch' parameter was appended at the end of the
|
||||
// existing signature, so existing subscribers needed no re-mapping.
|
||||
OnBeforePostDocument(SalesHeader, IsHandled, CalledFromBatch);
|
||||
// Subscribers bind by name, so the new parameter can sit between the
|
||||
// existing parameters without breaking subscribers that omit it.
|
||||
OnBeforePostDocument(SalesHeader, CalledFromBatch, IsHandled);
|
||||
if IsHandled then
|
||||
exit;
|
||||
end;
|
||||
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean; CalledFromBatch: Boolean)
|
||||
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean; var IsHandled: Boolean)
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
||||
codeunit 50252 "Existing Param Subscriber"
|
||||
{
|
||||
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Param Append Good Sample", 'OnBeforePostDocument', '', false, false)]
|
||||
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean)
|
||||
begin
|
||||
IsHandled := SalesHeader."No." = '';
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,26 +1,26 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: events
|
||||
keywords: [event-parameters, signature, backward-compatibility, append, onbefore, integration-event, versioning]
|
||||
keywords: [event-parameters, signature, backward-compatibility, public-event, local-event, internal-event, appsourcecop, as0024, as0025]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Add new event parameters at the end
|
||||
# Event parameter additions depend on publisher access, not position
|
||||
|
||||
## Description
|
||||
|
||||
Adding a parameter to an existing event publisher changes its signature. Appending the new parameter at the end of the parameter list keeps the change easy to review and track: existing subscribers still bind to the leading parameters, and the diff is a single clean addition. Inserting a parameter in the middle makes diffs noisy and harder to review, and obscures the history of how the signature evolved. New parameters belong after the existing ones.
|
||||
Event subscribers bind publisher parameters by name and can omit parameters they do not use. A `local` or `internal` Business or Integration event can therefore gain a parameter at any position without breaking subscriber-only consumers; appending is not a compatibility requirement. A public event is also a public procedure that dependent extensions can raise, so adding a required parameter anywhere breaks callers under AppSourceCop AS0024.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When extending an existing publisher, append the new parameter after all existing ones, including after a trailing `var IsHandled: Boolean` when present. Subscribers that already match keep working against the leading parameters, and the change stays a one-line addition that is trivial to review.
|
||||
Add a parameter directly only when the shipped event publisher is `local` or `internal`. Place it where the signature is clearest; existing subscribers continue binding the parameters they name. For a public event, keep the original publisher unchanged and introduce a new event with the expanded contract.
|
||||
|
||||
See sample: `add-new-event-parameters-at-the-end.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Inserting a new parameter in the middle of an existing event's signature, shifting every subsequent parameter and making the change noisy and harder to review. Detection: a changed event signature where an added parameter appears before existing parameters rather than at the tail of the list.
|
||||
Appending a parameter to a public event and assuming its position makes the change compatible. Existing external callers still lack the new required argument. Conversely, do not flag a parameter inserted among existing parameters on a `local` or `internal` Business or Integration event merely because it was not appended.
|
||||
|
||||
See sample: `add-new-event-parameters-at-the-end.bad.al`.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,38 @@
|
|||
// Demonstration only. Shows the wrong pattern: raising the integration event inside a TryFunction body.
|
||||
|
||||
codeunit 50116 "Payment Processor Bad"
|
||||
{
|
||||
[IntegrationEvent(false, false)]
|
||||
procedure OnBeforeSubmitPayment(var PaymentAmount: Decimal; var Cancel: Boolean)
|
||||
begin
|
||||
end;
|
||||
|
||||
procedure SubmitPayment(PaymentAmount: Decimal)
|
||||
var
|
||||
Success: Boolean;
|
||||
begin
|
||||
// TryFunction wraps both the event raise and the gateway call.
|
||||
Success := TrySubmitPaymentInternal(PaymentAmount);
|
||||
if not Success then
|
||||
Error('Payment gateway call failed. Check connectivity and retry.');
|
||||
end;
|
||||
|
||||
[TryFunction]
|
||||
local procedure TrySubmitPaymentInternal(PaymentAmount: Decimal)
|
||||
var
|
||||
Cancel: Boolean;
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
begin
|
||||
Cancel := false;
|
||||
// BAD: event raised inside TryFunction. Any Error() thrown by a subscriber is caught here
|
||||
// and silently swallowed - the subscriber's error never reaches the caller.
|
||||
// A subscriber setting Cancel := true is also lost when TryFunction returns false.
|
||||
OnBeforeSubmitPayment(PaymentAmount, Cancel);
|
||||
if Cancel then
|
||||
exit;
|
||||
Client.Get('https://payments.example.com/submit?amount=' + Format(PaymentAmount), Response);
|
||||
if not Response.IsSuccessStatusCode() then
|
||||
Error('HTTP %1', Response.HttpStatusCode());
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,38 @@
|
|||
// Demonstration only. Shows the correct pattern: raise the integration event before entering TryFunction.
|
||||
|
||||
codeunit 50114 "Payment Processor"
|
||||
{
|
||||
[IntegrationEvent(false, false)]
|
||||
procedure OnBeforeSubmitPayment(var PaymentAmount: Decimal; var Cancel: Boolean)
|
||||
begin
|
||||
end;
|
||||
|
||||
procedure SubmitPayment(PaymentAmount: Decimal)
|
||||
var
|
||||
Cancel: Boolean;
|
||||
Success: Boolean;
|
||||
begin
|
||||
Cancel := false;
|
||||
// Event raised outside the try scope - subscriber errors propagate normally to the caller.
|
||||
OnBeforeSubmitPayment(PaymentAmount, Cancel);
|
||||
if Cancel then
|
||||
exit;
|
||||
|
||||
// Only the operation that can fail transiently lives inside TryFunction.
|
||||
Success := TryCallPaymentGateway(PaymentAmount);
|
||||
if not Success then
|
||||
Error('Payment gateway call failed. Check connectivity and retry.');
|
||||
end;
|
||||
|
||||
[TryFunction]
|
||||
local procedure TryCallPaymentGateway(PaymentAmount: Decimal)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
begin
|
||||
// ... build request, set headers ...
|
||||
Client.Get('https://payments.example.com/submit?amount=' + Format(PaymentAmount), Response);
|
||||
if not Response.IsSuccessStatusCode() then
|
||||
Error('HTTP %1', Response.HttpStatusCode());
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: events
|
||||
keywords: [tryfunction, integration-event, subscriber, error-handling, silent-failure, event-publisher]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not raise integration events inside a TryFunction
|
||||
|
||||
## Description
|
||||
|
||||
A `TryFunction` catches all errors — including errors thrown by event subscribers. When an `[IntegrationEvent]` is raised inside a `TryFunction` body, any error a subscriber raises is silently swallowed by the TryFunction's error boundary. The subscriber's logic fails, the caller sees no error, and the calling code continues as if nothing happened. Subscribers have no way to signal failure to the caller.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Raise the integration event before entering the TryFunction scope. The event and its subscribers execute outside the error boundary, so subscriber errors propagate normally to the caller. Move only the operation that genuinely needs error isolation (such as an HTTP call or a posting step) inside the TryFunction.
|
||||
|
||||
See sample: `avoid-raising-events-inside-try-functions.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Raising an integration event inside a TryFunction body. Subscriber failures are caught and discarded by the TryFunction. The subscriber contract — that a subscriber can signal failure to the caller — is silently broken.
|
||||
|
||||
See sample: `avoid-raising-events-inside-try-functions.bad.al`.
|
||||
|
|
@ -8,9 +8,9 @@ codeunit 50291 "New OnBefore Bad Sample"
|
|||
begin
|
||||
Total := 100;
|
||||
|
||||
// Anti-pattern: IsHandled was bolted onto the existing
|
||||
// OnAfterCalculateTotal, changing its contract and breaking every
|
||||
// subscriber that matched the original signature.
|
||||
// Anti-pattern: IsHandled was bolted onto the existing OnAfter event.
|
||||
// Regardless of compiler compatibility, this changes a notification
|
||||
// into an override contract that existing subscribers did not expect.
|
||||
OnAfterCalculateTotal(SalesHeader, Total, IsHandled);
|
||||
end;
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,14 @@
|
|||
// Demonstration-only AL. Version 1 used [IntegrationEvent(true, true, false)].
|
||||
codeunit 50531 "Shipment Events Bad"
|
||||
{
|
||||
procedure NotifyShipment(ShipmentNo: Code[20])
|
||||
begin
|
||||
OnShipmentCreated(ShipmentNo);
|
||||
end;
|
||||
|
||||
// Version 2 mutates all three contract-significant arguments in place.
|
||||
[IntegrationEvent(false, false, true)]
|
||||
local procedure OnShipmentCreated(ShipmentNo: Code[20])
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,21 @@
|
|||
// Demonstration-only AL. The Isolated argument requires runtime 9.0 / BC20.
|
||||
codeunit 50530 "Shipment Events"
|
||||
{
|
||||
procedure NotifyShipment(ShipmentNo: Code[20])
|
||||
begin
|
||||
OnShipmentCreated(ShipmentNo);
|
||||
OnShipmentCreatedIsolated(ShipmentNo);
|
||||
end;
|
||||
|
||||
// Preserve the shipped attribute contract.
|
||||
[IntegrationEvent(true, true, false)]
|
||||
local procedure OnShipmentCreated(ShipmentNo: Code[20])
|
||||
begin
|
||||
end;
|
||||
|
||||
// Publish a new event for different isolation and sender semantics.
|
||||
[IntegrationEvent(false, false, true)]
|
||||
local procedure OnShipmentCreatedIsolated(ShipmentNo: Code[20])
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: events
|
||||
keywords: [event-attribute, includesender, globalvaraccess, isolated-event, compatibility, integration-event, business-event, appsourcecop, as0021, as0101]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not change shipped event attribute flags
|
||||
|
||||
## Description
|
||||
|
||||
`IncludeSender` and, on Integration events, `GlobalVarAccess` have been event-contract flags since runtime 1.0. Removing sender or global access breaks subscribers, so AppSourceCop AS0021 prevents changing those flags from `true` to `false`. On runtime 9.0 and later (Business Central 2022 release wave 1, BC20), `Isolated` also controls transaction, error, and rollback behavior; AS0101 prevents adding, removing, or changing that argument.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Keep every available attribute argument exactly as shipped. If new subscribers need different sender/global exposure, publish a new event with the desired flags. Apply the same rule to `Isolated` only on BC20 or later, where that argument exists. Raise both events while the original contract is supported, and choose preferred flags only when designing a new event.
|
||||
|
||||
See sample: `do-not-change-shipped-event-attribute-flags.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Changing a shipped event's `IncludeSender` or `GlobalVarAccess` to modernize its design, including replacing `IncludeSender` with an explicit parameter. On BC20 or later, adding, removing, or toggling `Isolated` is equally contract-significant. Even a change that leaves old subscribers compiling can alter observable execution or exposure; version the event instead.
|
||||
|
||||
See sample: `do-not-change-shipped-event-attribute-flags.bad.al`.
|
||||
|
|
@ -8,13 +8,13 @@ codeunit 50260 "Reuse Event Good Sample"
|
|||
IsHandled := false;
|
||||
// A single event, extended with CustomerNo appended at the end, covers
|
||||
// the need; no second event is raised beside it.
|
||||
OnBeforeProcessOrder(SalesHeader, CustomerNo, IsHandled);
|
||||
OnBeforeProcessOrder(SalesHeader, IsHandled, CustomerNo);
|
||||
if IsHandled then
|
||||
exit;
|
||||
end;
|
||||
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnBeforeProcessOrder(var SalesHeader: Record "Sales Header"; CustomerNo: Code[20]; var IsHandled: Boolean)
|
||||
local procedure OnBeforeProcessOrder(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean; CustomerNo: Code[20])
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,20 +7,20 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# Prefer this over IncludeSender in codeunit events
|
||||
# Prefer this over IncludeSender in new codeunit events
|
||||
|
||||
## Description
|
||||
|
||||
Some publishers set `IncludeSender` to `true` on `[IntegrationEvent]` or `[BusinessEvent]` so subscribers receive the publishing object as an implicit sender parameter. From Business Central 2024 release wave 2, a codeunit can instead pass itself explicitly with the `this` keyword as a normal, strongly-typed `Sender` parameter. Explicit passing is clearer at both the publisher and the subscriber: the sender appears in the signature, it is concretely typed to the publishing codeunit, and it avoids the implicit-parameter mechanics of `IncludeSender`. Reserve `IncludeSender = true` for cases where the sender genuinely cannot be passed explicitly. This guidance applies to code targeting Business Central 2024 release wave 2 or later, where the `this` keyword is available.
|
||||
When designing a new publisher, setting `IncludeSender` to `true` on `[IntegrationEvent]` or `[BusinessEvent]` gives subscribers the publishing object as an implicit sender parameter. From Business Central 2024 release wave 2, a codeunit can instead pass itself explicitly with the `this` keyword as a normal, strongly-typed `Sender` parameter. Explicit passing makes the sender visible and typed in the signature. This is new-event design guidance only: never change `IncludeSender` on an event that has already shipped.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Declare the publisher `[IntegrationEvent(false, false)]` with an explicit `Sender: Codeunit "…"` parameter and raise it with `this`, for example `OnBeforeProcessOrder(OrderNo, this);`. Subscribers then receive a typed sender they can call directly.
|
||||
For a new event, declare the publisher `[IntegrationEvent(false, false)]` with an explicit `Sender: Codeunit "…"` parameter and raise it with `this`, for example `OnBeforeProcessOrder(OrderNo, this);`. Subscribers then receive a typed sender they can call directly.
|
||||
|
||||
See sample: `prefer-this-over-includesender-in-codeunit-events.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Relying on `[IntegrationEvent(true, …)]` solely to hand subscribers the publisher instance, where a codeunit could pass `this` explicitly as a typed parameter. Detection: `IncludeSender = true` on a codeunit event whose only purpose is to expose the sender, in code targeting Business Central 2024 release wave 2 or later.
|
||||
Designing a new codeunit event with `[IntegrationEvent(true, …)]` solely to hand subscribers the publisher instance, where `this` could be passed explicitly as a typed parameter. Do not apply this rule by mutating a shipped event's attribute flags.
|
||||
|
||||
See sample: `prefer-this-over-includesender-in-codeunit-events.bad.al`.
|
||||
|
|
|
|||
|
|
@ -20,13 +20,14 @@ codeunit 50225 "Reservation Post Good Sample"
|
|||
var
|
||||
IsHandled: Boolean;
|
||||
begin
|
||||
IsHandled := false;
|
||||
OnBeforeReserve(ReservationEntry, IsHandled);
|
||||
if IsHandled then
|
||||
exit;
|
||||
|
||||
ReservationEntry.Reserved := true;
|
||||
ReservationEntry.Modify(true);
|
||||
if not IsHandled then begin
|
||||
ReservationEntry.Reserved := true;
|
||||
ReservationEntry.Modify(true);
|
||||
end;
|
||||
|
||||
// OnAfter reports completion whether a subscriber or the base body handled it.
|
||||
OnAfterReserve(ReservationEntry);
|
||||
end;
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,15 @@
|
|||
// Demonstration-only AL. Version 1 exposed var Score as an Integer.
|
||||
codeunit 50521 "Customer Scoring Events Bad"
|
||||
{
|
||||
procedure ScoreCustomer(CustomerNo: Code[20]; ScoreText: Text)
|
||||
begin
|
||||
OnCustomerScored(CustomerNo, ScoreText);
|
||||
end;
|
||||
|
||||
// 'local' limits raising, not subscription. Renaming Score to ScoreText,
|
||||
// changing its type, and removing var all break existing subscribers.
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnCustomerScored(CustomerNo: Code[20]; ScoreText: Text)
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,24 @@
|
|||
// Demonstration-only AL. Version 1 had CustomerNo and var Score parameters.
|
||||
codeunit 50520 "Customer Scoring Events"
|
||||
{
|
||||
procedure ScoreCustomer(CustomerNo: Code[20]; Reason: Text; var Score: Integer)
|
||||
begin
|
||||
OnCustomerScored(CustomerNo, Reason, Score);
|
||||
end;
|
||||
|
||||
// Adding Reason between existing parameters preserves subscriber bindings.
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnCustomerScored(CustomerNo: Code[20]; Reason: Text; var Score: Integer)
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
||||
codeunit 50522 "Existing Scoring Subscriber"
|
||||
{
|
||||
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Customer Scoring Events", 'OnCustomerScored', '', false, false)]
|
||||
local procedure OnCustomerScored(CustomerNo: Code[20]; var Score: Integer)
|
||||
begin
|
||||
if CustomerNo = '' then
|
||||
Score := 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: events
|
||||
keywords: [local-event, internal-event, event-subscriber, compatibility, access-modifier, integration-event, business-event, parameter-name, var-parameter, appsourcecop]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Treat local and internal events as subscriber contracts
|
||||
|
||||
## Description
|
||||
|
||||
The `local` and `internal` access modifiers on Business and Integration event publishers restrict who can raise the procedure; they do not prevent dependent extensions from subscribing. Once shipped, the event name and each existing parameter's name, type/subtype, and value-versus-`var` passing mode are compatibility contracts even when the publisher is not public. Parameter order is not a subscriber contract because subscribers bind the parameters they use by name. This differs from `[InternalEvent]`, which is module-only except for modules named by `internalsVisibleTo`.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Preserve a shipped Business or Integration event's identity and every existing parameter's name, type/subtype, and passing mode regardless of the procedure access modifier. AS0025 protects names and types, while AS0063 and AS0077 protect removal and addition of `var`. New parameters may be added at any position on a `local` or `internal` event because subscribers can omit them; public event procedures follow the stricter caller contract described by `add-new-event-parameters-at-the-end`.
|
||||
|
||||
See sample: `treat-local-and-internal-events-as-subscriber-contracts.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Renaming or removing an existing parameter, changing its type/subtype, or adding/removing its `var` modifier because the event publisher procedure is `local` or `internal`. AppSourceCop checks these subscriber-breaking changes because dependent event subscribers can still bind to the event. Reordering unchanged parameters, or inserting a new parameter among them, is not this anti-pattern.
|
||||
|
||||
See sample: `treat-local-and-internal-events-as-subscriber-contracts.bad.al`.
|
||||
|
|
@ -7,6 +7,7 @@ codeunit 50220 "Shipping Charge Good Sample"
|
|||
begin
|
||||
// Give extensions a sanctioned seam to replace the calculation, then
|
||||
// skip the default logic when a subscriber has handled it.
|
||||
IsHandled := false;
|
||||
OnBeforeCalculateShippingCharge(OrderAmount, Charge, IsHandled);
|
||||
if IsHandled then
|
||||
exit(Charge);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,16 @@
|
|||
// Demonstration-only AL. Version 1 shipped with only CalculateAmount().
|
||||
interface "I Shipping Quote Bad"
|
||||
{
|
||||
procedure CalculateAmount(): Decimal;
|
||||
|
||||
// Added in version 2: every existing implementer now fails to compile.
|
||||
procedure CalculateDeliveryDate(): Date;
|
||||
}
|
||||
|
||||
codeunit 50511 "Existing Shipping Quote" implements "I Shipping Quote Bad"
|
||||
{
|
||||
procedure CalculateAmount(): Decimal
|
||||
begin
|
||||
exit(10);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,23 @@
|
|||
// Demonstration-only AL. Interface inheritance requires runtime 14.0 / BC25.
|
||||
interface "I Shipping Quote"
|
||||
{
|
||||
procedure CalculateAmount(): Decimal;
|
||||
}
|
||||
|
||||
interface "I Shipping Quote V2" extends "I Shipping Quote"
|
||||
{
|
||||
procedure CalculateDeliveryDate(): Date;
|
||||
}
|
||||
|
||||
codeunit 50510 "Shipping Quote V2" implements "I Shipping Quote V2"
|
||||
{
|
||||
procedure CalculateAmount(): Decimal
|
||||
begin
|
||||
exit(10);
|
||||
end;
|
||||
|
||||
procedure CalculateDeliveryDate(): Date
|
||||
begin
|
||||
exit(Today() + 1);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
bc-version: [16..]
|
||||
domain: interfaces
|
||||
keywords: [published-interface, interface-method, breaking-change, interface-extends, versioned-interface, appsourcecop, as0066]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Extend published interfaces; do not edit them
|
||||
|
||||
## Description
|
||||
|
||||
Adding a method to a shipped interface changes the contract every implementing codeunit must satisfy. Implementers can live in dependent extensions, so the addition breaks code the interface publisher cannot update; AppSourceCop reports AS0066. Interface inheritance is available from runtime 14.0 (Business Central 2024 release wave 2, BC25), but the original interface must remain unchanged.
|
||||
|
||||
## Best Practice
|
||||
|
||||
On BC25 or later, declare a new interface that `extends` the published interface and add the new method there. Existing implementers remain valid for the original contract, while new implementers opt in to the extended contract. For targets BC16 through BC24, where interface inheritance is unavailable, publish a new or versioned sibling interface instead.
|
||||
|
||||
See sample: `extend-published-interfaces-dont-edit-them.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Adding a procedure directly to an interface that has already shipped. Every dependent implementation must immediately add that procedure, so an otherwise compatible app update breaks its implementers.
|
||||
|
||||
See sample: `extend-published-interfaces-dont-edit-them.bad.al`.
|
||||
|
|
@ -0,0 +1,36 @@
|
|||
// Demonstration-only AL. A removed enum-extension value left ordinal 700 in data.
|
||||
enum 50503 "Delivery Method Bad" implements "I Delivery Method Bad"
|
||||
{
|
||||
Extensible = true;
|
||||
DefaultImplementation = "I Delivery Method Bad" = "Default Delivery Method Bad";
|
||||
|
||||
value(0; Default)
|
||||
{
|
||||
}
|
||||
}
|
||||
|
||||
interface "I Delivery Method Bad"
|
||||
{
|
||||
procedure Deliver();
|
||||
}
|
||||
|
||||
codeunit 50504 "Default Delivery Method Bad" implements "I Delivery Method Bad"
|
||||
{
|
||||
procedure Deliver()
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
||||
codeunit 50505 "Delivery Dispatch Bad"
|
||||
{
|
||||
procedure DeliverPersistedValue()
|
||||
var
|
||||
DeliveryMethod: Enum "Delivery Method Bad";
|
||||
Delivery: Interface "I Delivery Method Bad";
|
||||
begin
|
||||
DeliveryMethod := 700;
|
||||
// DefaultImplementation does not handle an ordinal that is not declared.
|
||||
Delivery := DeliveryMethod;
|
||||
Delivery.Deliver();
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,34 @@
|
|||
// Demonstration-only AL. UnknownValueImplementation requires runtime 7.0 / BC18.
|
||||
interface "I Delivery Method"
|
||||
{
|
||||
procedure Deliver();
|
||||
}
|
||||
|
||||
codeunit 50500 "Unknown Delivery Method" implements "I Delivery Method"
|
||||
{
|
||||
procedure Deliver()
|
||||
begin
|
||||
Error(UnknownMethodErr);
|
||||
end;
|
||||
|
||||
var
|
||||
UnknownMethodErr: Label 'The saved delivery method is no longer installed. Select another method.';
|
||||
}
|
||||
|
||||
codeunit 50501 "Default Delivery Method" implements "I Delivery Method"
|
||||
{
|
||||
procedure Deliver()
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
||||
enum 50502 "Delivery Method" implements "I Delivery Method"
|
||||
{
|
||||
Extensible = true;
|
||||
DefaultImplementation = "I Delivery Method" = "Default Delivery Method";
|
||||
UnknownValueImplementation = "I Delivery Method" = "Unknown Delivery Method";
|
||||
|
||||
value(0; Default)
|
||||
{
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
bc-version: [18..]
|
||||
domain: interfaces
|
||||
keywords: [unknownvalueimplementation, unknown-enum-value, persisted-ordinal, enum-extension, extension-uninstall, interface-fallback]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Handle unknown enum ordinals with UnknownValueImplementation
|
||||
|
||||
## Description
|
||||
|
||||
An enum ordinal can remain in persisted data after the enum extension that declared it is uninstalled. The ordinal is then unknown: it matches no currently declared enum value. `DefaultImplementation` does not cover this case; it covers declared values that have no explicit interface implementation. `UnknownValueImplementation`, available from runtime 7.0 (Business Central 2021 release wave 1, BC18), provides the distinct interface implementation for an unknown ordinal.
|
||||
|
||||
## Best Practice
|
||||
|
||||
On BC18 or later, set `UnknownValueImplementation = <Interface> = <Codeunit>;` on an enum that implements an interface and can be persisted. Use an implementation that reports a clear domain error or safely contains the unknown state. Keep `DefaultImplementation` separately when declared but unmapped values also need a fallback.
|
||||
|
||||
See sample: `handle-unknown-enum-ordinals-with-unknownvalueimplementation.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Defining only `DefaultImplementation` and assuming it also handles a stored ordinal whose enum value has disappeared. After an enum extension is uninstalled, converting that unknown ordinal to the interface can produce a technical runtime error instead of controlled handling.
|
||||
|
||||
See sample: `handle-unknown-enum-ordinals-with-unknownvalueimplementation.bad.al`.
|
||||
|
|
@ -11,11 +11,11 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
An `enum` that `implements` an interface maps each value to a codeunit through the `Implementation` property. But an extensible enum can carry values that set no `Implementation` — values added later by an extension, or a value left intentionally blank. Assigning such a value to an interface variable and calling a method on it fails at runtime unless the enum provides a fallback. The enum-level `DefaultImplementation` property names the codeunit used whenever a value has no explicit `Implementation`, so resolution always yields a usable object. LLMs are generally unaware this property exists and leave the gap open.
|
||||
An `enum` that `implements` an interface maps each declared value to a codeunit through the `Implementation` property. A declared value, including one supplied by an enum extension, can omit that mapping. Assigning that value to an interface variable then fails at runtime unless the enum provides `DefaultImplementation`. This property is for declared but unmapped values; an ordinal that is no longer declared is a different case covered by `handle-unknown-enum-ordinals-with-unknownvalueimplementation`.
|
||||
|
||||
## Best Practice
|
||||
|
||||
On any extensible enum that implements an interface, set `DefaultImplementation = <Interface> = <Codeunit>;` at the enum level, pointing at a safe implementation that does nothing harmful. Values with their own `Implementation` keep using it; every other value — including ones added later by extensions — resolves to the default instead of failing. For the distinct case of an out-of-range integer that matches no declared value, pair it with `UnknownValueImplementation`. The result is that a consumer can assign any enum value to the interface variable and call through it without a runtime guard.
|
||||
On any extensible enum that implements an interface, set `DefaultImplementation = <Interface> = <Codeunit>;` at the enum level, pointing at a safe implementation. Values with their own `Implementation` keep using it; declared values without one resolve to the default. Do not rely on this property for persisted ordinals that match no declared enum value.
|
||||
|
||||
See sample: `set-defaultimplementation-on-enum.good.al`.
|
||||
|
||||
|
|
|
|||
|
|
@ -2,13 +2,22 @@ report 50221 "Perf Sample AddLoadFields Bad"
|
|||
{
|
||||
dataset
|
||||
{
|
||||
// No AddLoadFields: every Cust. Ledger Entry column ships per row, even though
|
||||
// only three columns feed the layout.
|
||||
dataitem(CustLedgerEntry; "Cust. Ledger Entry")
|
||||
{
|
||||
column(CustomerNo; "Customer No.") { }
|
||||
column(PostingDate; "Posting Date") { }
|
||||
column(Amount; Amount) { }
|
||||
|
||||
trigger OnAfterGetRecord()
|
||||
begin
|
||||
// Source Code is not a dataset column, so its first access causes a
|
||||
// just-in-time load and updates the dataitem enumerator.
|
||||
RegisterSourceCode("Source Code");
|
||||
end;
|
||||
}
|
||||
}
|
||||
|
||||
local procedure RegisterSourceCode(SourceCode: Code[10])
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,8 +10,19 @@ report 50220 "Perf Sample AddLoadFields Good"
|
|||
|
||||
trigger OnPreDataItem()
|
||||
begin
|
||||
AddLoadFields("Customer No.", "Posting Date", Amount);
|
||||
// Dataset columns are selected by the report compiler. Source Code is
|
||||
// extra because only trigger code reads it.
|
||||
CustLedgerEntry.AddLoadFields("Source Code");
|
||||
end;
|
||||
|
||||
trigger OnAfterGetRecord()
|
||||
begin
|
||||
RegisterSourceCode("Source Code");
|
||||
end;
|
||||
}
|
||||
}
|
||||
|
||||
local procedure RegisterSourceCode(SourceCode: Code[10])
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,20 +7,20 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# In reports, declare the fields the layout needs with AddLoadFields
|
||||
# Add trigger-only report fields in OnPreDataItem
|
||||
|
||||
## Description
|
||||
|
||||
Reports iterate dataitems on potentially large source tables and pipe rows into a layout. The partial-record optimization is the same idea as `use-setloadfields-for-partial-records.md`, but the API is different: per the upstream guidance, "for reports, use `AddLoadFields()` in `OnPreDataItem` trigger to add fields needed by the layout." `AddLoadFields` is additive — call it for each field the layout consumes — and runs once per dataitem before iteration begins.
|
||||
Report dataitem field selection is calculated at compile time and once per dataitem type during execution. Fields referenced by dataset columns are selected automatically; fields used only in triggers are not. Use `AddLoadFields` in `OnPreDataItem` to supplement the automatic selection with normal fields that trigger code needs.
|
||||
|
||||
## Best Practice
|
||||
|
||||
In each dataitem's `OnPreDataItem` trigger, list the columns the layout binds to via `AddLoadFields(<field>, <field>, ...)`. The platform then materializes only those columns per row. Treat the layout column list as the spec: every column the layout uses must be added; columns the layout does not use should not be added.
|
||||
When a dataitem trigger needs an extra field, add that field in `OnPreDataItem` before iteration starts. This supplements the compiler-selected fields and avoids the first just-in-time load and enumerator update when the trigger reads the extra field.
|
||||
|
||||
See sample: `addloadfields-in-report-onpredataitem.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Relying on the dataitem's default to load every field. On a report bound to a ledger-scale table this transfers an entire row per iteration, of which the layout reads a fraction.
|
||||
Listing every dataset column in `AddLoadFields`, or omitting a known trigger-only field because the dataset already uses other fields. The former is redundant; the latter causes a just-in-time load on first access and can cause repeated loads when the record is copied or passed by value.
|
||||
|
||||
See sample: `addloadfields-in-report-onpredataitem.bad.al`.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,19 @@
|
|||
codeunit 50493 "Perf Record Clone Bad"
|
||||
{
|
||||
procedure IncreaseCustomerCreditLimits(Percent: Decimal)
|
||||
var
|
||||
Customer: Record Customer;
|
||||
CustomerCopy: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||
Customer.SetFilter("Credit Limit (LCY)", '>0');
|
||||
if Customer.FindSet(true) then
|
||||
repeat
|
||||
CustomerCopy.Copy(Customer);
|
||||
CustomerCopy.Validate(
|
||||
"Credit Limit (LCY)",
|
||||
Round(CustomerCopy."Credit Limit (LCY)" * (1 + Percent / 100)));
|
||||
CustomerCopy.Modify(true);
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,17 @@
|
|||
codeunit 50492 "Perf Record Clone Good"
|
||||
{
|
||||
procedure IncreaseCustomerCreditLimits(Percent: Decimal)
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||
Customer.SetFilter("Credit Limit (LCY)", '>0');
|
||||
if Customer.FindSet(true) then
|
||||
repeat
|
||||
Customer.Validate(
|
||||
"Credit Limit (LCY)",
|
||||
Round(Customer."Credit Limit (LCY)" * (1 + Percent / 100)));
|
||||
Customer.Modify(true);
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [clone, clone-before-write, copy, gettable, by-value, copied-record, writing-helper]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Avoid cloning records before Modify or Delete in loops
|
||||
|
||||
## Description
|
||||
|
||||
Microsoft's [AL database-method performance guidance](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/administration/optimize-sql-al-database-methods-and-performance-on-server#insert-modify-delete-and-locktable) states that cloning an iterated record before `Modify` or `Delete` restarts the SQL `SELECT` and issues an extra SQL statement for every row. The runtime treats `Record.Copy`, `RecordRef.GetTable`, and passing a record by value to a writing helper as clones in this situation.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Use `FindSet(true)` when the loop writes the traversed rows, and call `Modify` or `Delete` on that iterating record variable. If generic code is required, open and iterate the `RecordRef` directly instead of calling `GetTable` for each typed record. Keep a per-row loop when validation or row-specific behavior is required; this rule does not imply that `ModifyAll` or `DeleteAll` is equivalent.
|
||||
|
||||
See sample: `avoid-cloning-records-before-modify-delete-in-loops.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Inside an active traversal, copy the current row, convert it with `RecordRef.GetTable`, or pass it without `var` to a helper, then call `Modify` or `Delete` on that clone. Do not flag read-only snapshots, temporary records, or copies used to write a different target table; the documented extra-statement concern is clone-before-write on the traversed table.
|
||||
|
||||
See sample: `avoid-cloning-records-before-modify-delete-in-loops.bad.al`.
|
||||
|
|
@ -1,21 +1,60 @@
|
|||
query 50127 "Perf Customer Chunk"
|
||||
{
|
||||
QueryType = Normal;
|
||||
OrderBy = ascending(CustomerNo);
|
||||
|
||||
elements
|
||||
{
|
||||
dataitem(Customer; Customer)
|
||||
{
|
||||
column(CustomerNo; "No.") { }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50128 "Perf Sample CommitInLoop Good"
|
||||
{
|
||||
procedure NormalizeCustomerNames()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
RowsInChunk: Integer;
|
||||
ChunkSize: Integer;
|
||||
LastCustomerNo: Code[20];
|
||||
begin
|
||||
ChunkSize := 500;
|
||||
if Customer.FindSet(true) then
|
||||
// The outer loop owns checkpoints; the per-row loop contains no Commit.
|
||||
while NormalizeNextChunk(LastCustomerNo) do
|
||||
Commit();
|
||||
end;
|
||||
|
||||
local procedure NormalizeNextChunk(var LastCustomerNo: Code[20]): Boolean
|
||||
var
|
||||
Customer: Record Customer;
|
||||
TempCustomer: Record Customer temporary;
|
||||
CustomerChunk: Query "Perf Customer Chunk";
|
||||
LastChunkCustomerNo: Code[20];
|
||||
begin
|
||||
CustomerChunk.TopNumberOfRows(500);
|
||||
if LastCustomerNo <> '' then
|
||||
CustomerChunk.SetFilter(CustomerNo, '>%1', LastCustomerNo);
|
||||
CustomerChunk.Open();
|
||||
while CustomerChunk.Read() do begin
|
||||
TempCustomer.Init();
|
||||
TempCustomer."No." := CustomerChunk.CustomerNo;
|
||||
TempCustomer.Insert();
|
||||
LastChunkCustomerNo := CustomerChunk.CustomerNo;
|
||||
end;
|
||||
CustomerChunk.Close();
|
||||
|
||||
if TempCustomer.IsEmpty() then
|
||||
exit(false);
|
||||
|
||||
Customer.LockTable();
|
||||
if TempCustomer.FindSet() then
|
||||
repeat
|
||||
Customer.Name := UpperCase(Customer.Name);
|
||||
Customer.Modify();
|
||||
RowsInChunk += 1;
|
||||
if RowsInChunk >= ChunkSize then begin
|
||||
Commit();
|
||||
RowsInChunk := 0;
|
||||
if Customer.Get(TempCustomer."No.") then begin
|
||||
Customer.Name := UpperCase(Customer.Name);
|
||||
Customer.Modify();
|
||||
end;
|
||||
until Customer.Next() = 0;
|
||||
until TempCustomer.Next() = 0;
|
||||
|
||||
LastCustomerNo := LastChunkCustomerNo;
|
||||
exit(true);
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [commit, loop, transaction, lock, checkpoint, codeunit-run]
|
||||
keywords: [commit, commit-in-loop, per-row-commit, checkpoint, bounded-checkpoint, watermark, topnumberofrows]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
|
|
@ -13,17 +13,16 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
Commit ends the current write transaction. Calling it inside a per-row loop produces one transaction per iteration and loses the ability to roll back the whole operation atomically; it also interferes with the platform's ability to batch write operations. Most loops need no explicit Commit at all — AL auto-commits the enclosing code module on successful completion (see `understand-implicit-transaction-boundary.md`). When the batch is too large for one transaction, the fix is not a per-row Commit but bounded checkpoints that each process N rows.
|
||||
Commit ends the current write transaction. Calling it inside a per-row loop produces one transaction per iteration and loses the ability to roll back the whole operation atomically; it also interferes with the platform's ability to batch write operations. Most loops need no explicit Commit at all — AL auto-commits the enclosing code module on successful completion (see `understand-implicit-transaction-boundary.md`). When the batch is too large for one transaction, the fix is not a per-row Commit but bounded checkpoints that select an exact list of at most N keys and process only those rows.
|
||||
|
||||
## Best Practice
|
||||
|
||||
If the batch is large enough that a single transaction is untenable, process it in checkpoints driven by an outer loop that each time picks up the next N rows. Commit once per checkpoint at a clearly defined safe boundary, not inside the per-row loop. Wrapping each chunk in `Codeunit.Run` gives the same effect with native rollback on failure — see `codeunit-run-as-atomic-sub-operation.md`.
|
||||
If the batch is large enough that a single transaction is untenable, use an ordered primary-key watermark and retrieve a bounded next-N key list. `FindSet` is optimized for reading the complete filtered set and isn't implemented as `TOP X`, so calling it over the remaining tail and breaking after N rows does not bound retrieval. The sample uses a query capped by [`TopNumberOfRows`](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/query/queryinstance-topnumberofrows-method) to fill a temporary key buffer, then takes update locks and modifies only those exact keys. It does not reconstruct an inclusive first-to-last range that concurrent inserts could expand. Commit after the bounded inner loop returns and persist its last selected key as the next watermark. Use a stable key and define how a later run handles records inserted at or below an already committed watermark. A `Codeunit.Run` boundary can also own a chunk when its implicit commit and error behavior fit the caller — see `codeunit-run-as-atomic-sub-operation.md`.
|
||||
|
||||
See sample: `avoid-commit-inside-loops.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Placing Commit inside `repeat ... until Next() = 0` is almost always a mistake: it is unusual for the correctness of the operation to depend on per-row commits, and the cost of starting a new transaction on every row dominates the work.
|
||||
Placing Commit inside `repeat ... until Next() = 0` is almost always a mistake: it is unusual for the correctness of the operation to depend on per-row commits, and the cost of starting a new transaction on every row dominates the work. A capped query that discovers only an upper key and then re-reads an inclusive key range is not exact batching either; concurrent inserts inside that range can enlarge the checkpoint.
|
||||
|
||||
See sample: `avoid-commit-inside-loops.bad.al`.
|
||||
|
||||
|
|
|
|||
|
|
@ -1,15 +1,17 @@
|
|||
codeunit 50253 "Perf Sample NPlus1 Bad"
|
||||
{
|
||||
procedure SumStdCost(var BOMLine: Record "BOM Component") TotalCost: Decimal
|
||||
procedure SumStdCost(BOMNo: Code[20]; BOMVersionCode: Code[20]) TotalCost: Decimal
|
||||
var
|
||||
BOMLine: Record "Production BOM Line";
|
||||
Item: Record Item;
|
||||
begin
|
||||
BOMLine.SetRange("Production BOM No.", BOMNo);
|
||||
BOMLine.SetRange("Version Code", BOMVersionCode);
|
||||
if BOMLine.FindSet() then
|
||||
repeat
|
||||
// Full-row Item.Get per BOM line — no partial loading, no caching.
|
||||
Item.Get(BOMLine."No.");
|
||||
if Item."Costing Method" = Item."Costing Method"::Standard then
|
||||
TotalCost += Item."Standard Cost" * BOMLine."Quantity per";
|
||||
if Item.Get(BOMLine."No.") then
|
||||
if Item."Costing Method" = Item."Costing Method"::Standard then
|
||||
TotalCost += Item."Standard Cost" * BOMLine."Quantity per";
|
||||
until BOMLine.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,15 +1,38 @@
|
|||
codeunit 50252 "Perf Sample NPlus1 Good"
|
||||
query 50252 "Perf Sample BOM Cost"
|
||||
{
|
||||
procedure SumStdCost(var BOMLine: Record "BOM Component") TotalCost: Decimal
|
||||
QueryType = Normal;
|
||||
|
||||
elements
|
||||
{
|
||||
dataitem(ProductionBOMLine; "Production BOM Line")
|
||||
{
|
||||
column(ProductionBOMNo; "Production BOM No.") { }
|
||||
column(VersionCode; "Version Code") { }
|
||||
column(QuantityPer; "Quantity per") { }
|
||||
|
||||
dataitem(Item; Item)
|
||||
{
|
||||
DataItemLink = "No." = ProductionBOMLine."No.";
|
||||
DataItemTableFilter = "Costing Method" = const(Standard);
|
||||
SqlJoinType = InnerJoin;
|
||||
|
||||
column(StandardCost; "Standard Cost") { }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50254 "Perf Sample NPlus1 Good"
|
||||
{
|
||||
procedure SumStdCost(BOMNo: Code[20]; BOMVersionCode: Code[20]) TotalCost: Decimal
|
||||
var
|
||||
Item: Record Item;
|
||||
BOMCost: Query "Perf Sample BOM Cost";
|
||||
begin
|
||||
Item.SetLoadFields("Costing Method", "Standard Cost");
|
||||
if BOMLine.FindSet() then
|
||||
repeat
|
||||
if Item.Get(BOMLine."No.") then
|
||||
if Item."Costing Method" = Item."Costing Method"::Standard then
|
||||
TotalCost += Item."Standard Cost" * BOMLine."Quantity per";
|
||||
until BOMLine.Next() = 0;
|
||||
BOMCost.SetRange(ProductionBOMNo, BOMNo);
|
||||
BOMCost.SetRange(VersionCode, BOMVersionCode);
|
||||
BOMCost.Open();
|
||||
while BOMCost.Read() do
|
||||
TotalCost += BOMCost.StandardCost * BOMCost.QuantityPer;
|
||||
BOMCost.Close();
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
A `Get` or `FindFirst` against a different record inside a loop body produces one database round-trip per iteration — the classic N+1 pattern. Per the upstream guidance, "Flag when a `Get()`/`FindFirst()` is called inside a loop for each record — this creates N+1 database round-trips." The cost only matters when the inner table is meaningful: lookups against temporary tables, singleton setup tables, enum-mapping tables, permission objects, or Role IDs are bounded and safe. The pattern to catch is the inner lookup that hits a production-scale table for every outer row.
|
||||
A `Get` or `FindFirst` against another persistent table inside a loop can produce an N+1 access pattern: one outer query followed by repeated inner lookups. Server and primary-key caches can satisfy some `Get` calls, so a source-level `Get` is not proof of one SQL round-trip. The concern is an unbounded loop whose lookup keys are not known to repeat or remain cached.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When the loop needs values from another record, lift the lookup out of the loop if the rows can be collected up front, or apply `SetLoadFields` so each inner read transfers only the columns the loop actually uses (see `use-setloadfields-for-partial-records.md`). When the inner record is small or bounded, leave the call site alone — the rule targets large-table inner lookups specifically.
|
||||
Use a query object to join the outer and inner tables when the relationship and filters can be expressed as one query. If keys repeat, a dictionary cache can reduce lookups to one per distinct key. `SetLoadFields` can reduce the columns transferred by unavoidable inner reads, but it does not eliminate the N+1 shape and must not be presented as doing so.
|
||||
|
||||
See sample: `avoid-get-inside-loop-on-large-table.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Iterating BOM lines and calling `Item.Get(BOMLine."No.")` per row to read a costing method, with no `SetLoadFields` on `Item`. Each iteration issues one query against Item (~800k rows) and pulls the entire row to read two fields. The fix is `Item.SetLoadFields("Costing Method", "Standard Cost");` ahead of the loop — still N reads, but each one transfers only the needed columns.
|
||||
Iterating production BOM lines and calling `Item.Get(BOMLine."No.")` for each line when the same result can be produced by a query joining Production BOM Line to Item. Partial loading alone is only a payload mitigation for this pattern.
|
||||
|
||||
See sample: `avoid-get-inside-loop-on-large-table.bad.al`.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,16 @@
|
|||
codeunit 50100 "Event Audit Buffer"
|
||||
{
|
||||
SingleInstance = true;
|
||||
|
||||
// Unbounded global: every event fires adds an entry for the lifetime of the session.
|
||||
var
|
||||
AllEventIds: List of [Guid];
|
||||
|
||||
[EventSubscriber(ObjectType::Table, Database::"Sales Header", OnAfterInsertEvent, '', false, false)]
|
||||
local procedure OnAfterInsertSalesHeader(var Rec: Record "Sales Header")
|
||||
begin
|
||||
// No cap. No eviction. No reset. A session that sees ten thousand inserts
|
||||
// keeps ten thousand GUIDs in memory until the user signs out.
|
||||
AllEventIds.Add(Rec.SystemId);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
codeunit 50100 "Event Audit Buffer"
|
||||
{
|
||||
SingleInstance = true;
|
||||
|
||||
var
|
||||
RecentEventIds: List of [Guid];
|
||||
MaxBuffered: Integer;
|
||||
|
||||
trigger OnRun()
|
||||
begin
|
||||
MaxBuffered := 50;
|
||||
end;
|
||||
|
||||
[EventSubscriber(ObjectType::Table, Database::"Sales Header", OnAfterInsertEvent, '', false, false)]
|
||||
local procedure OnAfterInsertSalesHeader(var Rec: Record "Sales Header")
|
||||
begin
|
||||
// Bounded cache: drop the oldest entry when the cap is reached.
|
||||
RecentEventIds.Add(Rec.SystemId);
|
||||
if RecentEventIds.Count() > MaxBuffered then
|
||||
RecentEventIds.RemoveAt(1);
|
||||
end;
|
||||
|
||||
procedure ResetAtBusinessProcessBoundary()
|
||||
begin
|
||||
// Explicit reset point at a natural boundary in the workflow.
|
||||
Clear(RecentEventIds);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [singleinstance, subscriber, event, memory, session]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Avoid growing globals in SingleInstance subscribers
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
A codeunit with `SingleInstance = true` is allocated once per session and lives until the session ends. Global variables on it are never collected between event fires. A subscriber that accumulates data into a global — buffering payloads, appending to a list, caching without a cap — steadily grows its session footprint for the entire user session. The symptom is memory that only recovers on sign-out, and it surfaces only on long-running sessions.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Keep the global footprint on a SingleInstance subscriber bounded and intentional: a handful of flags, a setup record, a bounded cache with a maximum size. When cross-event state is genuinely needed, define an explicit reset point — end of a business process, arrival of a specific terminal event — that clears the growing collection.
|
||||
|
||||
See sample: `avoid-growing-globals-in-singleinstance-subscribers.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A SingleInstance subscriber that appends each event's payload to a global list, dictionary, or temporary record without a cap or cleanup trigger. The list grows for hours, memory pressure builds quietly, and debugging the root cause on a live environment is substantially harder than noticing the unbounded append in code review.
|
||||
|
||||
See sample: `avoid-growing-globals-in-singleinstance-subscribers.bad.al`.
|
||||
|
|
@ -0,0 +1,32 @@
|
|||
table 50100 "Item Ledger Entry (Demo)"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer) { DataClassification = SystemMetadata; }
|
||||
field(2; "Item No."; Code[20]) { DataClassification = CustomerContent; }
|
||||
field(3; "Posting Date"; Date) { DataClassification = CustomerContent; }
|
||||
field(4; Quantity; Decimal) { DataClassification = CustomerContent; }
|
||||
field(5; "Cost Amount"; Decimal) { DataClassification = CustomerContent; }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.") { Clustered = true; }
|
||||
|
||||
// Write-heavy ledger key: aggregates on this key are read rarely relative
|
||||
// to INSERT frequency. Keeping SIFT live on every write is net-negative.
|
||||
key(ByItemAndDate; "Item No.", "Posting Date")
|
||||
{
|
||||
SumIndexFields = Quantity, "Cost Amount";
|
||||
MaintainSIFTIndex = false;
|
||||
}
|
||||
|
||||
// Dashboard-facing key: aggregates read on every session load, underlying
|
||||
// rows updated infrequently. Keeping SIFT live pays for itself.
|
||||
key(ByItem; "Item No.")
|
||||
{
|
||||
SumIndexFields = Quantity;
|
||||
MaintainSIFTIndex = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [maintainsiftindex, sift, calcsums, flowfield, write-cost]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Choose MaintainSIFTIndex by read-write ratio
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
`MaintainSIFTIndex` on a key decides whether SQL Server maintains the SIFT indexed view as underlying rows change. With `Yes`, writes that affect the key or sum fields also maintain the indexed aggregate. With `No`, that SIFT indexed view is not maintained, so a compatible `CalcSums` or FlowField calculation is computed from the base table instead and may require scanning many rows. There is no deferred "first read rebuild" of the SIFT structure.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Measure aggregate-read latency and write cost under realistic filters and volumes. Keep `MaintainSIFTIndex = true` when the maintained aggregate materially benefits frequent `CalcSums` or FlowField reads. Consider `false` when writes dominate and the less-frequent aggregate reads can tolerate calculation from the base table.
|
||||
|
||||
See sample: `choose-maintainsiftindex-by-read-write-ratio.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Leaving `MaintainSIFTIndex = Yes` on every key by reflex or convenience. On write-heavy tables the cumulative cost turns every INSERT or MODIFY into several additional aggregate updates, and the impact compounds in batch imports and posting routines — often without any code-review signal that the property is the cause.
|
||||
|
|
@ -0,0 +1,30 @@
|
|||
codeunit 50100 "Sales Document Processor"
|
||||
{
|
||||
procedure DescribeDocument(DocumentType: Enum "Sales Document Type"; DocumentNo: Code[20]): Text
|
||||
var
|
||||
SalesHeader: Record "Sales Header";
|
||||
begin
|
||||
SalesHeader.SetLoadFields(
|
||||
"Sell-to Customer No.",
|
||||
"Order Date", "Shipment Date", "Completely Shipped",
|
||||
"Posting Date", "Due Date", "Payment Terms Code");
|
||||
SalesHeader.Get(DocumentType, DocumentNo);
|
||||
|
||||
case DocumentType of
|
||||
DocumentType::Order:
|
||||
exit(DescribeOrder(SalesHeader));
|
||||
DocumentType::Invoice:
|
||||
exit(DescribeInvoice(SalesHeader));
|
||||
end;
|
||||
end;
|
||||
|
||||
local procedure DescribeOrder(SalesHeader: Record "Sales Header"): Text
|
||||
begin
|
||||
exit(StrSubstNo('%1|%2|%3|%4', SalesHeader."Sell-to Customer No.", SalesHeader."Order Date", SalesHeader."Shipment Date", SalesHeader."Completely Shipped"));
|
||||
end;
|
||||
|
||||
local procedure DescribeInvoice(SalesHeader: Record "Sales Header"): Text
|
||||
begin
|
||||
exit(StrSubstNo('%1|%2|%3|%4', SalesHeader."Sell-to Customer No.", SalesHeader."Posting Date", SalesHeader."Due Date", SalesHeader."Payment Terms Code"));
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,34 @@
|
|||
codeunit 50100 "Sales Document Processor"
|
||||
{
|
||||
procedure DescribeDocument(DocumentType: Enum "Sales Document Type"; DocumentNo: Code[20]): Text
|
||||
var
|
||||
SalesHeader: Record "Sales Header";
|
||||
begin
|
||||
SalesHeader.SetLoadFields("Sell-to Customer No.");
|
||||
|
||||
case DocumentType of
|
||||
DocumentType::Order:
|
||||
begin
|
||||
SalesHeader.AddLoadFields("Order Date", "Shipment Date", "Completely Shipped");
|
||||
SalesHeader.Get(DocumentType, DocumentNo);
|
||||
exit(DescribeOrder(SalesHeader));
|
||||
end;
|
||||
DocumentType::Invoice:
|
||||
begin
|
||||
SalesHeader.AddLoadFields("Posting Date", "Due Date", "Payment Terms Code");
|
||||
SalesHeader.Get(DocumentType, DocumentNo);
|
||||
exit(DescribeInvoice(SalesHeader));
|
||||
end;
|
||||
end;
|
||||
end;
|
||||
|
||||
local procedure DescribeOrder(SalesHeader: Record "Sales Header"): Text
|
||||
begin
|
||||
exit(StrSubstNo('%1|%2|%3|%4', SalesHeader."Sell-to Customer No.", SalesHeader."Order Date", SalesHeader."Shipment Date", SalesHeader."Completely Shipped"));
|
||||
end;
|
||||
|
||||
local procedure DescribeInvoice(SalesHeader: Record "Sales Header"): Text
|
||||
begin
|
||||
exit(StrSubstNo('%1|%2|%3|%4', SalesHeader."Sell-to Customer No.", SalesHeader."Posting Date", SalesHeader."Due Date", SalesHeader."Payment Terms Code"));
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [setloadfields, case, conditional, branch, field-loading]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Load common fields before branching on case
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
When a known input determines which fields a subsequent record read will use, a single `SetLoadFields` containing every branch's fields loads unnecessary columns. Build the selection before `Get`, `FindFirst`, or `FindSet`: use `SetLoadFields` for fields common to every branch, then `AddLoadFields` for the selected branch. `SetLoadFields` replaces the current selection, while `AddLoadFields` preserves it.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Call `SetLoadFields` with the common fields. In each branch, call `AddLoadFields` with that branch's normal fields and then perform the record read. This applies only when the discriminator is known before the read; branching on a field from an already-loaded row is too late to tailor that row's initial SQL projection.
|
||||
|
||||
See sample: `load-common-fields-before-branching-on-case.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A single top-level `SetLoadFields` enumerating every branch's fields, or a branch-local `SetLoadFields` that accidentally discards the common selection. Both make the declared load plan differ from the fields the selected path actually uses.
|
||||
|
||||
See sample: `load-common-fields-before-branching-on-case.bad.al`.
|
||||
|
|
@ -0,0 +1,18 @@
|
|||
codeunit 50100 "Item Reindex Queue"
|
||||
{
|
||||
procedure QueueItemsForReindex(CategoryCode: Code[20])
|
||||
var
|
||||
Item: Record Item;
|
||||
ReindexQueue: Codeunit "Reindex Queue";
|
||||
begin
|
||||
// Default full-record load. Description, Unit Price, Inventory, and
|
||||
// every other column are fetched across the wire and held in memory
|
||||
// for the whole loop - the body only ever reads "No.".
|
||||
Item.SetRange("Item Category Code", CategoryCode);
|
||||
|
||||
if Item.FindSet() then
|
||||
repeat
|
||||
ReindexQueue.Enqueue(Item."No.");
|
||||
until Item.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,17 @@
|
|||
codeunit 50100 "Item Reindex Queue"
|
||||
{
|
||||
procedure QueueItemsForReindex(CategoryCode: Code[20])
|
||||
var
|
||||
Item: Record Item;
|
||||
ReindexQueue: Codeunit "Reindex Queue";
|
||||
begin
|
||||
// Only the primary key is used in the loop body; load nothing else.
|
||||
Item.SetLoadFields("No.");
|
||||
Item.SetRange("Item Category Code", CategoryCode);
|
||||
|
||||
if Item.FindSet() then
|
||||
repeat
|
||||
ReindexQueue.Enqueue(Item."No.");
|
||||
until Item.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [setloadfields, primary-key, reference, existence-check, memory]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Load only primary key fields for reference work
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Work that uses a record only for its identity — passing it to another procedure that will re-fetch what it needs, queueing a key for later processing, running existence checks, or building a reference collection — does not need non-key payload fields. `SetLoadFields` with only the primary key fields loads the minimum that preserves record identity while skipping everything else. On wide tables with large text, BLOB, or media fields the difference in memory and transfer is substantial.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When the iterating code's body touches only primary key fields (or passes the record to another procedure that will apply its own `SetLoadFields`), declare `SetLoadFields` with just the primary key fields before applying filters and calling `FindSet`. Callers downstream that need more fields issue their own `Get` or extend the load explicitly.
|
||||
|
||||
See sample: `load-only-primary-key-fields-for-reference-work.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Using the default full-record load in loops whose body only reads the primary key, or forwards the record to another codeunit that immediately re-queries. The non-key payload is fetched across the wire and held in memory for the duration of the loop, then discarded unread.
|
||||
|
||||
See sample: `load-only-primary-key-fields-for-reference-work.bad.al`.
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
codeunit 50100 "Document Router"
|
||||
{
|
||||
procedure Route(SalesHeader: Record "Sales Header")
|
||||
begin
|
||||
// Alphabetical ordering. Every Order (the ~85% common case) evaluates
|
||||
// "Credit Memo", "Invoice", and "Quote" before matching.
|
||||
case SalesHeader."Document Type" of
|
||||
SalesHeader."Document Type"::"Credit Memo":
|
||||
RouteCreditMemo(SalesHeader);
|
||||
SalesHeader."Document Type"::Invoice:
|
||||
RouteInvoice(SalesHeader);
|
||||
SalesHeader."Document Type"::Quote:
|
||||
RouteQuote(SalesHeader);
|
||||
SalesHeader."Document Type"::Order:
|
||||
RouteOrder(SalesHeader);
|
||||
SalesHeader."Document Type"::"Return Order":
|
||||
RouteReturnOrder(SalesHeader);
|
||||
end;
|
||||
end;
|
||||
|
||||
local procedure RouteOrder(SalesHeader: Record "Sales Header") begin end;
|
||||
local procedure RouteInvoice(SalesHeader: Record "Sales Header") begin end;
|
||||
local procedure RouteQuote(SalesHeader: Record "Sales Header") begin end;
|
||||
local procedure RouteCreditMemo(SalesHeader: Record "Sales Header") begin end;
|
||||
local procedure RouteReturnOrder(SalesHeader: Record "Sales Header") begin end;
|
||||
}
|
||||
|
|
@ -0,0 +1,25 @@
|
|||
codeunit 50100 "Document Router"
|
||||
{
|
||||
procedure Route(SalesHeader: Record "Sales Header")
|
||||
begin
|
||||
// Profiling shows Orders are the common case, so that branch goes first.
|
||||
case SalesHeader."Document Type" of
|
||||
SalesHeader."Document Type"::Order:
|
||||
RouteOrder(SalesHeader);
|
||||
SalesHeader."Document Type"::Invoice:
|
||||
RouteInvoice(SalesHeader);
|
||||
SalesHeader."Document Type"::"Credit Memo":
|
||||
RouteCreditMemo(SalesHeader);
|
||||
SalesHeader."Document Type"::Quote:
|
||||
RouteQuote(SalesHeader);
|
||||
SalesHeader."Document Type"::"Return Order":
|
||||
RouteReturnOrder(SalesHeader);
|
||||
end;
|
||||
end;
|
||||
|
||||
local procedure RouteOrder(SalesHeader: Record "Sales Header") begin end;
|
||||
local procedure RouteInvoice(SalesHeader: Record "Sales Header") begin end;
|
||||
local procedure RouteQuote(SalesHeader: Record "Sales Header") begin end;
|
||||
local procedure RouteCreditMemo(SalesHeader: Record "Sales Header") begin end;
|
||||
local procedure RouteReturnOrder(SalesHeader: Record "Sales Header") begin end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [case, branch, frequency, control-flow, hot-path]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Order case branches by frequency
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
AL documentation does not guarantee that a `case` statement uses a linear comparison strategy, so branch frequency alone is not proof of a performance issue. Reordering is justified only when profiling on the target runtime shows that a large, heavily skewed `case` is a material hot path. It is not a default review finding.
|
||||
|
||||
## Best Practice
|
||||
|
||||
After profiling confirms the comparison path matters and the runtime frequency is known, list common branches first without changing the set of handled values, fallback behavior, or branch bodies.
|
||||
|
||||
See sample: `order-case-branches-by-frequency.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Reordering branches based on assumed frequency without profiling, or changing an `else` arm or handled value while making the optimization. The good and bad forms must differ only in branch order.
|
||||
|
||||
See sample: `order-case-branches-by-frequency.bad.al`.
|
||||
|
|
@ -11,12 +11,12 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
A temporary table supports a full record API — filters, iteration, multi-field keys — but a pure key→value lookup pays for plumbing it does not use. Per the upstream guidance, "if a temporary table record is ONLY used as a lookup table, it is faster to use a dictionary which supports O(1) lookups instead of O(lg n) for temporary tables." The Dictionary type has no record machinery to traverse; the key hash answers the lookup directly.
|
||||
An AL `Dictionary` directly models an unordered unique key-to-value collection. A temporary table models records and supports keys, filters, validation, and ordered iteration in Business Central Server memory. For a pure lookup map, the dictionary avoids repeatedly configuring and searching a temporary record and makes the intended access pattern explicit.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When the use of a temp record is "set a key, see if the row exists, read a single value", switch to `Dictionary of [Key, Value]`. Use the temp-table form when the use genuinely needs filtering, iteration in a specific order, or a multi-field key. Compatibility with code that expects a `Record` parameter is a real reason to keep the temp table; performance alone, on a pure lookup, is not.
|
||||
Use `Dictionary of [Key, Value]` when the operation is add-or-replace, contains-key, and get-value by one supported key type. Use a temporary table when the value is a record, or when the code needs filters, ordered iteration, multiple fields, multiple keys, or table behavior. Both structures consume service-tier memory and still need volume analysis.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A temp `Record` declared, populated row by row, then queried with `SetRange(KeyField, X); if Find('=') then Value := Rec.ValueField;`. The lookup hashes the key behind the scenes and does the same work a `Dictionary` would, plus the per-row record overhead. The pattern often appears because the author originally needed iteration and the iteration was later removed without revisiting the data structure.
|
||||
A temporary record used only through `SetRange(KeyField, X); FindFirst()` to retrieve one scalar value, with no record semantics that justify the table. The opposite mistake is replacing a temporary table that needs filtering or ordered iteration with a dictionary.
|
||||
|
|
|
|||
|
|
@ -1,15 +1,29 @@
|
|||
table 50243 "Perf Import Staging Entry"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer) { }
|
||||
field(2; "Batch ID"; Guid) { }
|
||||
field(3; Processed; Boolean) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50243 "Perf Sample ModifyAll Bad"
|
||||
{
|
||||
procedure ApplyPriceUpdate(NewPrice: Decimal)
|
||||
procedure MarkBatchProcessed(BatchId: Guid)
|
||||
var
|
||||
SalesLine: Record "Sales Line";
|
||||
StagingEntry: Record "Perf Import Staging Entry";
|
||||
begin
|
||||
SalesLine.SetRange(Type, SalesLine.Type::Item);
|
||||
// N writes when one ModifyAll would do.
|
||||
if SalesLine.FindSet() then
|
||||
StagingEntry.SetRange("Batch ID", BatchId);
|
||||
if StagingEntry.FindSet(true) then
|
||||
repeat
|
||||
SalesLine.Validate("Unit Price", NewPrice);
|
||||
SalesLine.Modify(true);
|
||||
until SalesLine.Next() = 0;
|
||||
StagingEntry.Processed := true;
|
||||
StagingEntry.Modify(false);
|
||||
until StagingEntry.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,20 +1,26 @@
|
|||
table 50242 "Perf Import Staging Entry"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer) { }
|
||||
field(2; "Batch ID"; Guid) { }
|
||||
field(3; Processed; Boolean) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50242 "Perf Sample ModifyAll Good"
|
||||
{
|
||||
procedure ApplyPriceUpdate(NewPrice: Decimal)
|
||||
procedure MarkBatchProcessed(BatchId: Guid)
|
||||
var
|
||||
SalesLine: Record "Sales Line";
|
||||
StagingEntry: Record "Perf Import Staging Entry";
|
||||
begin
|
||||
SalesLine.SetRange(Type, SalesLine.Type::Item);
|
||||
SalesLine.ModifyAll("Unit Price", NewPrice);
|
||||
end;
|
||||
|
||||
procedure ApplyTolerance(DocumentNo: Code[20]; ToleranceAmount: Decimal)
|
||||
var
|
||||
CustLedgerEntry: Record "Cust. Ledger Entry";
|
||||
begin
|
||||
CustLedgerEntry.SetRange("Document No.", DocumentNo);
|
||||
CustLedgerEntry.SetRange(Open, true);
|
||||
CustLedgerEntry.ModifyAll("Accepted Payment Tolerance", ToleranceAmount);
|
||||
CustLedgerEntry.ModifyAll("Accepted Pmt. Disc. Tolerance", false);
|
||||
StagingEntry.SetRange("Batch ID", BatchId);
|
||||
// Processed has no OnValidate logic, and the equivalent loop uses Modify(false).
|
||||
StagingEntry.ModifyAll(Processed, true, false);
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,20 +7,20 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use ModifyAll / DeleteAll instead of per-row Modify / Delete in a loop
|
||||
# Use ModifyAll only for equivalent bulk assignments
|
||||
|
||||
## Description
|
||||
|
||||
`ModifyAll` and `DeleteAll` are the bulk APIs. Per the upstream guidance, they "execute as single SQL statements" when the table supports it — one round-trip updates or deletes every row in the filtered set. The anti-pattern is the loop equivalent: `FindSet` followed by per-row `Modify`/`Delete`, where the runtime issues one write per row. On a production-scale table the difference is the difference between a single statement and N statements.
|
||||
`ModifyAll` assigns one value to one field across the filtered set. It does not run the field's `OnValidate` trigger. Its optional `RunTrigger` parameter controls the table `OnModify` trigger, not field validation. Replacing a loop is therefore correct only when direct assignment is semantically equivalent for every row.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When the loop body does nothing more than assign a constant value (or a value computed once) to one or more fields, replace the loop with `ModifyAll("Field 1", Value1)` — and chain additional `ModifyAll` calls for additional fields. The same shape applies to `DeleteAll`. Be aware that the bulk APIs can regress to row-by-row execution for tables with certain trigger or media-field configurations (see `triggers-and-media-field-regress-modifyall.md`); when that regression applies, multiple `ModifyAll` calls become more expensive than one manual loop, so the choice is conditional, not absolute.
|
||||
Use `ModifyAll` when the loop directly assigns the same value, does not call `Validate`, needs no per-row calculation, and does not depend on `OnModify` unless the equivalent `RunTrigger` value is supplied. Check whether table-extension triggers, event subscribers, global triggers, or media fields force row-by-row fallback (see `triggers-and-media-field-regress-modifyall.md`).
|
||||
|
||||
See sample: `prefer-modifyall-over-per-row-modify.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`if SalesLine.FindSet() then repeat SalesLine.Validate("Unit Price", NewPrice); SalesLine.Modify(true); until SalesLine.Next() = 0;` — N writes when one would do. The pattern is easy to introduce when the loop initially does per-row computation and is later simplified to assign a constant; the loop scaffolding survives the simplification.
|
||||
A loop that only assigns a constant and calls `Modify(false)` on a field with no validation side effects. Conversely, replacing `Validate(Field, Value); Modify(true)` with `ModifyAll(Field, Value)` is also an anti-pattern because it silently drops field validation and may drop table-trigger behavior.
|
||||
|
||||
See sample: `prefer-modifyall-over-per-row-modify.bad.al`.
|
||||
|
|
|
|||
|
|
@ -1,13 +1,13 @@
|
|||
codeunit 50233 "Perf Sample ReadIso Bad"
|
||||
{
|
||||
procedure GetOrCreate(var AgentStatus: Record "Agent Status")
|
||||
procedure IsCustomerBlocked(CustomerNo: Code[20]): Boolean
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
// LockTable poisons every subsequent read of Agent Status in the
|
||||
// surrounding transaction with UPDLOCK — even for callers that only read.
|
||||
AgentStatus.LockTable();
|
||||
if not AgentStatus.Get() then begin
|
||||
AgentStatus.Init();
|
||||
AgentStatus.Insert();
|
||||
end;
|
||||
Customer.LockTable();
|
||||
if not Customer.Get(CustomerNo) then
|
||||
exit(false);
|
||||
|
||||
exit(Customer.Blocked <> Customer.Blocked::" ");
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,11 +1,13 @@
|
|||
codeunit 50232 "Perf Sample ReadIso Good"
|
||||
{
|
||||
procedure GetOrCreate(var AgentStatus: Record "Agent Status")
|
||||
procedure IsCustomerBlocked(CustomerNo: Code[20]): Boolean
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
AgentStatus.ReadIsolation := IsolationLevel::ReadCommitted;
|
||||
if not AgentStatus.Get() then begin
|
||||
AgentStatus.Init();
|
||||
AgentStatus.Insert();
|
||||
end;
|
||||
Customer.ReadIsolation := IsolationLevel::ReadCommitted;
|
||||
if not Customer.Get(CustomerNo) then
|
||||
exit(false);
|
||||
|
||||
exit(Customer.Blocked <> Customer.Blocked::" ");
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
`LockTable` and `ReadIsolation` solve different problems with different blast radii. Per the upstream guidance, "`LockTable` ensures that all READS against that table will happen with UPDLOCK for the remainder of the transaction." `ReadIsolation` "only pertains to the current record instance, while `LockTable` affects the lockstate of the entire transaction." `ReadIsolation` is also more expressive: it can heighten or lower the isolation level inside an already-established transaction. Reaching for `LockTable` when only a single read needs guarding therefore poisons every later read on that table — including reads in other code paths that share the transaction.
|
||||
Without read scale-out, `LockTable` causes subsequent reads of that table in the transaction to use `UPDLOCK`. With read scale-out, those reads use `REPEATABLEREAD` on the replica instead. `ReadIsolation` selects an isolation level for one record instance. A helper that only reads should not broaden locking for the table merely to request committed data.
|
||||
|
||||
## Best Practice
|
||||
|
||||
For a read-only operation, or a single read that needs a higher isolation level than the surrounding transaction, set `Rec.ReadIsolation := IsolationLevel::ReadCommitted;` (or the level the call requires) immediately before the read. The hint applies only to that record instance. Save `LockTable` for code that genuinely needs every subsequent read on the table to acquire an update lock (see `findset-true-applies-updlock-on-read.md` for the alternative narrower mechanism on iterated reads).
|
||||
For a read-only operation that specifically requires committed data, set `Rec.ReadIsolation := IsolationLevel::ReadCommitted` immediately before the read. If the default isolation is sufficient, set neither property. `ReadCommitted` can still block behind writers and does not guarantee that repeated reads stay unchanged; use the isolation level required by the operation. Reserve update locks for read-before-write logic, not read-only helpers.
|
||||
|
||||
See sample: `prefer-readisolation-over-locktable-for-reads.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`Rec.LockTable();` at the top of a helper that only reads, perhaps to "make sure the read is consistent". Every subsequent read on that table for the rest of the transaction acquires `UPDLOCK`, including reads from unrelated code paths fused into the same transaction. The contention surfaces in unrelated user sessions, not in the helper that introduced it.
|
||||
`Rec.LockTable();` at the top of a helper that only reads, perhaps to "make sure the read is consistent". It takes stronger isolation than the helper needs and changes later reads of that table in the surrounding transaction or read-scale-out session.
|
||||
|
||||
See sample: `prefer-readisolation-over-locktable-for-reads.bad.al`.
|
||||
|
|
|
|||
|
|
@ -1,22 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [table-size, hot-table, ledger-entry, item, customer, sales-line, scale]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Production-scale tables warrant concrete performance analysis
|
||||
|
||||
## Description
|
||||
|
||||
Some Business Central tables routinely reach sizes where access patterns matter much more than they do on a generic table. The upstream review guidance lists ten of them with P95 row counts: Item (~800k), Customer (~800k), Item Ledger Entry (~10M), Value Entry (~10M), G/L Entry (~10M), VAT Entry (~10M), Customer Ledger Entry (~10M), Vendor Ledger Entry (~10M), Sales Invoice Header (~300k), and Sales Invoice Line (~3M). These figures are not platform constants — they are the volumes a reviewer should assume when judging a change.
|
||||
|
||||
## Best Practice
|
||||
|
||||
For any code change that touches one of these tables, do not approve the pattern on intuition. Walk through the SQL the change implies (one query? one per row? one per chunk?), the memory it allocates (a `List` per row?), and the CPU work per row, against the row counts above. Smaller tables can tolerate a sub-optimal access pattern; these cannot. The rest of this domain — `apply-filters-before-iterating.md`, `use-setloadfields-for-partial-records.md`, `avoid-calcfields-in-loops.md`, `pair-findset-with-next-loop.md`, `avoid-get-inside-loop-on-persistent-tables.md` — exists primarily so that code touching these tables stays on the safe side of each rule.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Generalizing from a unit test or a development tenant. A `FindSet` loop with a per-row `CalcFields` may execute in milliseconds against a few thousand rows on a developer's machine and become a multi-minute table scan against ten million Value Entry rows in production. Reasoning about performance from the dev-tenant timing instead of the production volume is the single most common way a regression ships.
|
||||
|
|
@ -7,16 +7,16 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# Singleton setup tables hold one row; access-pattern optimization is wasted
|
||||
# Enforced singleton setup tables need no access optimization
|
||||
|
||||
## Description
|
||||
|
||||
Business Central setup tables — `Sales & Receivables Setup`, `General Ledger Setup`, `FA Setup`, `Purchases & Payables Setup`, and the broader pattern of any `*Setup` table — hold at most one record per company. Per the upstream guidance, "any access pattern is fine, no `SetLoadFields` needed" on these tables. The same applies to other small bounded tables (enum mappings, permission objects, Role IDs) and system metadata tables (`TableMetadata`, `Field`, `AllObjWithCaption`) where iteration is safe.
|
||||
An access-pattern exemption is valid only for a table whose schema and write paths enforce at most one row for the relevant scope. A conventional blank primary key, a parameterless `Get()`, or a table name ending in `Setup` does not enforce that invariant; another primary-key value can still create another row unless insertion logic prevents it.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Skip access-pattern optimization on singleton-setup-style tables. `SalesReceivablesSetup.Get()` does not need `SetLoadFields` (see `use-setloadfields-for-partial-records.md`); a `repeat ... until` over a permission-object table does not need bulk operations. Spend the review attention on the production-scale tables instead (see `production-scale-tables-warrant-extra-analysis.md`).
|
||||
Exempt a setup read only after confirming that noncanonical keys are rejected and every supported creation path preserves the singleton. Otherwise apply ordinary access-pattern analysis, even when existing application code normally uses one blank-key record.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Mechanically applying the rules in this domain to every `Record` variable in the codebase. Flagging "missing `SetLoadFields`" on `GeneralLedgerSetup` or "use `IsEmpty` instead of `FindSet`" on a setup table adds noise without payoff — the optimization saves nothing measurable on a one-row table — and trains readers to ignore the review channel.
|
||||
Treating every `*Setup` table or parameterless `Get()` as proof of bounded cardinality without checking the primary key and insertion logic.
|
||||
|
|
|
|||
|
|
@ -7,16 +7,16 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# Temporary tables are in-memory; access-pattern rules do not apply
|
||||
# Temporary tables avoid SQL I/O, not in-memory work
|
||||
|
||||
## Description
|
||||
|
||||
A record declared `Temporary` (or a page with `SourceTableTemporary = true`) lives entirely in memory; reads and writes never reach SQL. Per the upstream guidance, "any access pattern (FindSet, FindFirst, Get, loops) on temp tables is acceptable — they are in-memory and fast." The rules in the rest of this domain — partial loading, bulk operations, N+1 detection, `IsEmpty` over `Count` — exist to avoid database round-trips that a temporary table does not perform.
|
||||
A temporary table stores its rows in Business Central Server memory instead of a physical SQL table. Its reads and writes therefore do not incur SQL round-trips, locking, or SIFT maintenance. They still allocate memory and execute record filtering, key lookup, sorting, insertion, and iteration in the service tier; those costs grow with the temporary dataset and access pattern.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Recognize the `Temporary` property (on a record variable, table declaration, or page's `SourceTableTemporary`) and exempt the code from access-pattern flags. The `SetLoadFields`/`FindSet` discipline that matters for `Customer` does not matter for a temporary `Customer` variable used as a working set. The interesting performance question on a temp table is volume in memory, not query plan.
|
||||
Do not apply SQL-specific findings such as missing `SetLoadFields`, lock contention, or N+1 database round-trips to a temporary record. Still assess memory volume and repeated scans or lookups. For a pure key-to-value collection, consider an AL `Dictionary`; keep a temporary table when record fields, keys, filtering, or ordered iteration are required.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Flagging a temporary table's `FindFirst` inside a loop, or a temporary table without `SetLoadFields`, as a performance issue. The recommendation produces no measurable gain and obscures genuine issues elsewhere in the same review.
|
||||
Claiming that every temporary-table access pattern is free because no SQL is involved. A nested scan over a large in-memory buffer can still dominate service-tier CPU, while adding `SetLoadFields` to that buffer addresses a database cost that does not exist.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,29 @@
|
|||
table 50100 "Perf Import Buffer"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer) { }
|
||||
field(2; "Batch ID"; Guid) { }
|
||||
field(3; Payload; Blob) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.") { Clustered = true; }
|
||||
key(ByBatch; "Batch ID") { }
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50100 "Perf Import Buffer Cleanup"
|
||||
{
|
||||
procedure ClearBatch(BatchId: Guid)
|
||||
var
|
||||
ImportBuffer: Record "Perf Import Buffer";
|
||||
begin
|
||||
ImportBuffer.SetRange("Batch ID", BatchId);
|
||||
if ImportBuffer.FindSet() then
|
||||
repeat
|
||||
ImportBuffer.Delete(false);
|
||||
until ImportBuffer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
table 50100 "Perf Import Buffer"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer) { }
|
||||
field(2; "Batch ID"; Guid) { }
|
||||
field(3; Payload; Blob) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.") { Clustered = true; }
|
||||
key(ByBatch; "Batch ID") { }
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50100 "Perf Import Buffer Cleanup"
|
||||
{
|
||||
procedure ClearBatch(BatchId: Guid)
|
||||
var
|
||||
ImportBuffer: Record "Perf Import Buffer";
|
||||
begin
|
||||
ImportBuffer.SetRange("Batch ID", BatchId);
|
||||
// This staging table has no base delete trigger. Installed extensions and
|
||||
// subscribers must also be checked before assuming the set-based fast path.
|
||||
ImportBuffer.DeleteAll(false);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [deleteall, bulk-delete, sql, ondelete, trigger-bypass]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use DeleteAll for filtered bulk deletion
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
`DeleteAll(false)` is eligible for a set-based SQL delete with the record variable's filters applied. It is not guaranteed to stay one statement. The base table `OnDelete` trigger is skipped, but table-extension `OnBeforeDelete` and `OnAfterDelete` triggers still run. Extension event subscribers, global delete triggers, and media fields can also require row processing. `DeleteAll(true)` runs the base table `OnDelete` trigger as well and has no performance advantage over `Delete(true)` in a loop.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Use filtered `DeleteAll(false)` for purpose-built staging or cleanup tables only after verifying that base-table `OnDelete` logic is unnecessary and installed extensions, subscribers, global triggers, and media fields do not add required per-row behavior or regress the bulk path. If deletion requires per-row business logic, keep an explicit triggered operation instead of simulating trigger execution separately.
|
||||
|
||||
See sample: `use-deleteall-for-filtered-bulk-deletion.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Iterating with `FindSet` + `Delete(false)` to clear a filtered staging batch that has no delete logic. The reverse mistake is assuming `DeleteAll` is always one SQL statement without checking table extensions and subscribers.
|
||||
|
||||
See sample: `use-deleteall-for-filtered-bulk-deletion.bad.al`.
|
||||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Add table
Add a link
Reference in a new issue