mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Add 12 general AL event-design articles to events domain
Add 12 atomic knowledge articles under microsoft/knowledge/events covering general AL event-design best practices: IsHandled initialization and OnAfter preservation, appending new event parameters, position-based event naming, reusing/extending events, avoiding per-iteration publishing, Temp-prefixing temporary record parameters, unabbreviated parameter names, preferring the this keyword over IncludeSender, avoiding loosely typed parameters, not mutating existing event contracts, and not bypassing critical operations with IsHandled. Each article ships a .good.al and .bad.al demonstration sample (object IDs 50240-50296; not compiled by CI). Extend the al-events-review leaf Worklist with one targeted check per new rule. Additive only; no contract or wiring change (events leaf already wired). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
54ddd8ecc2
commit
faeacb2484
37 changed files with 900 additions and 1 deletions
|
|
@ -0,0 +1,21 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50251 "Param Append Bad Sample"
|
||||||
|
{
|
||||||
|
procedure PostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
||||||
|
var
|
||||||
|
IsHandled: Boolean;
|
||||||
|
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;
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,20 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50250 "Param Append Good Sample"
|
||||||
|
{
|
||||||
|
procedure PostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
||||||
|
var
|
||||||
|
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);
|
||||||
|
if IsHandled then
|
||||||
|
exit;
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean; CalledFromBatch: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: events
|
||||||
|
keywords: [event-parameters, signature, backward-compatibility, append, onbefore, integration-event, versioning]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Add new event parameters at the end
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Adding a parameter to an existing event publisher changes its signature, and every subscriber must be updated to match. Appending the new parameter at the end of the parameter list keeps the change easy to review and minimizes churn: existing subscribers still bind to the leading parameters, and the diff is a single clean addition. Inserting a parameter in the middle shifts every following argument, makes diffs noisy, and is error-prone to reconcile across many subscribers — a subscriber that compiles can still receive the wrong values because positions moved. New parameters belong after the existing ones.
|
||||||
|
|
||||||
|
## 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.
|
||||||
|
|
||||||
|
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 the position of every subsequent argument and forcing a careful re-map of all subscribers. Detection: a changed event signature where an added parameter appears before existing parameters rather than at the tail of the list.
|
||||||
|
|
||||||
|
See sample: `add-new-event-parameters-at-the-end.bad.al`.
|
||||||
|
|
@ -0,0 +1,18 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50286 "Typed Param Bad Sample"
|
||||||
|
{
|
||||||
|
procedure ValidateQuantity(var SalesLine: Record "Sales Line"; xSalesLine: Record "Sales Line")
|
||||||
|
var
|
||||||
|
RecRef: RecordRef;
|
||||||
|
begin
|
||||||
|
// Anti-pattern: a RecordRef drops the table type and xRec is ambiguous
|
||||||
|
// out of context, so subscribers lose type safety and a clear contract.
|
||||||
|
RecRef.GetTable(SalesLine);
|
||||||
|
OnAfterValidateQuantity(RecRef, xSalesLine);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterValidateQuantity(var RecRef: RecordRef; xSalesLine: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,14 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50285 "Typed Param Good Sample"
|
||||||
|
{
|
||||||
|
procedure ValidateQuantity(var SalesLine: Record "Sales Line"; PreviousQuantity: Decimal)
|
||||||
|
begin
|
||||||
|
// A concrete record plus the specific value needed: type-safe contract.
|
||||||
|
OnAfterValidateQuantity(SalesLine, PreviousQuantity);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterValidateQuantity(var SalesLine: Record "Sales Line"; PreviousQuantity: Decimal)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: events
|
||||||
|
keywords: [recordref, xrec, type-safety, event-parameters, strong-typing, integration-event, clarity]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Avoid loosely typed event parameters
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Passing `RecordRef` or `xRec` as event parameters weakens the contract. A `RecordRef` parameter erases the table type, so subscribers must inspect at run time which table they received and can be handed an unexpected one, losing compile-time checking and direct field access. `xRec` — the previous version of a record — is context-dependent: it is meaningful inside a specific table or page trigger, but ambiguous once passed around as a parameter, and is often stale or empty outside the context that produced it. Prefer a concrete, strongly-typed record plus the specific values a subscriber actually needs, so the contract is explicit and the compiler enforces it.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Give events concrete record types and explicit values, such as `(SalesLine: Record "Sales Line"; PreviousQuantity: Decimal)`, instead of a `RecordRef` or an `xRec` parameter. Subscribers then get type safety, field access, and an unambiguous contract.
|
||||||
|
|
||||||
|
See sample: `avoid-loosely-typed-event-parameters.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Event parameters typed as `RecordRef` (no table type) or an `xRec`-style "previous record" (ambiguous, possibly stale) without strong justification. Detection: an event signature containing a `RecordRef` parameter, or a passed-through `xRec` record, where a concrete typed record and explicit values would serve.
|
||||||
|
|
||||||
|
See sample: `avoid-loosely-typed-event-parameters.bad.al`.
|
||||||
|
|
@ -0,0 +1,21 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50291 "New OnBefore Bad Sample"
|
||||||
|
{
|
||||||
|
procedure CalculateTotal(var SalesHeader: Record "Sales Header")
|
||||||
|
var
|
||||||
|
Total: Decimal;
|
||||||
|
IsHandled: Boolean;
|
||||||
|
begin
|
||||||
|
Total := 100;
|
||||||
|
|
||||||
|
// Anti-pattern: IsHandled was bolted onto the existing
|
||||||
|
// OnAfterCalculateTotal, changing its contract and breaking every
|
||||||
|
// subscriber that matched the original signature.
|
||||||
|
OnAfterCalculateTotal(SalesHeader, Total, IsHandled);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterCalculateTotal(var SalesHeader: Record "Sales Header"; Total: Decimal; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,28 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50290 "New OnBefore Good Sample"
|
||||||
|
{
|
||||||
|
procedure CalculateTotal(var SalesHeader: Record "Sales Header")
|
||||||
|
var
|
||||||
|
Total: Decimal;
|
||||||
|
IsHandled: Boolean;
|
||||||
|
begin
|
||||||
|
// New overridable seam added as a separate event; the existing
|
||||||
|
// OnAfterCalculateTotal keeps its original signature and subscribers.
|
||||||
|
IsHandled := false;
|
||||||
|
OnBeforeCalculateTotal(SalesHeader, IsHandled);
|
||||||
|
if not IsHandled then
|
||||||
|
Total := 100;
|
||||||
|
|
||||||
|
OnAfterCalculateTotal(SalesHeader, Total);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeCalculateTotal(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterCalculateTotal(var SalesHeader: Record "Sales Header"; Total: Decimal)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: events
|
||||||
|
keywords: [ishandled, breaking-change, event-contract, backward-compatibility, onbefore, integration-event, subscribers]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Do not add IsHandled to an existing event
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Adding a `var IsHandled: Boolean` parameter to an event that already shipped without one is a breaking contract change. The signature changes, so existing subscribers no longer match and silently stop firing until they are updated, and the event's meaning shifts from "notify" to "overridable" — a semantic the original subscribers never agreed to. The safe move is to leave the existing event untouched and introduce a new `OnBefore…` event carrying `IsHandled` at the point you want to make overridable. Existing subscribers keep working against the original event; new subscribers opt into the override seam through the new one.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Keep the existing event as-is and add a separate `OnBeforeX(…; var IsHandled: Boolean)` before the logic you want to make overridable. Two events with distinct, stable contracts are safer than one event whose meaning and signature were changed under its subscribers.
|
||||||
|
|
||||||
|
See sample: `do-not-add-ishandled-to-an-existing-event.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Mutating a shipped event — for example adding `var IsHandled` to `OnAfterCalculateTotal` — to retrofit override behaviour, breaking every existing subscriber and overloading the event's meaning. Detection: an `IsHandled` parameter added to a pre-existing event signature rather than introduced through a new dedicated `OnBefore` publisher.
|
||||||
|
|
||||||
|
See sample: `do-not-add-ishandled-to-an-existing-event.bad.al`.
|
||||||
|
|
@ -0,0 +1,30 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50296 "Critical Op Bad Sample"
|
||||||
|
{
|
||||||
|
procedure PostInvoice(var SalesHeader: Record "Sales Header")
|
||||||
|
var
|
||||||
|
IsHandled: Boolean;
|
||||||
|
begin
|
||||||
|
// Anti-pattern: IsHandled wraps the entire posting. A subscriber can set
|
||||||
|
// IsHandled := true and silently skip ledger-entry creation and the
|
||||||
|
// status update, leaving imbalanced ledgers and orphaned documents.
|
||||||
|
IsHandled := false;
|
||||||
|
OnBeforePostInvoice(SalesHeader, IsHandled);
|
||||||
|
if IsHandled then
|
||||||
|
exit;
|
||||||
|
|
||||||
|
CreateCustomerLedgerEntry(SalesHeader);
|
||||||
|
SalesHeader.Status := SalesHeader.Status::Released;
|
||||||
|
SalesHeader.Modify(true);
|
||||||
|
end;
|
||||||
|
|
||||||
|
local procedure CreateCustomerLedgerEntry(var SalesHeader: Record "Sales Header")
|
||||||
|
begin
|
||||||
|
// Posts the customer ledger entry (critical; must never be skipped).
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforePostInvoice(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,38 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50295 "Critical Op Good Sample"
|
||||||
|
{
|
||||||
|
procedure PostInvoice(var SalesHeader: Record "Sales Header")
|
||||||
|
var
|
||||||
|
DiscountAmount: Decimal;
|
||||||
|
IsHandled: Boolean;
|
||||||
|
begin
|
||||||
|
// IsHandled guards only a safe, side-effect-free calculation.
|
||||||
|
IsHandled := false;
|
||||||
|
OnBeforeCalculateInvoiceDiscount(SalesHeader, DiscountAmount, IsHandled);
|
||||||
|
if not IsHandled then
|
||||||
|
DiscountAmount := 10;
|
||||||
|
SalesHeader."Invoice Discount Amount" := DiscountAmount;
|
||||||
|
|
||||||
|
// Critical operations always run; no subscriber can bypass them.
|
||||||
|
CreateCustomerLedgerEntry(SalesHeader);
|
||||||
|
SalesHeader.Status := SalesHeader.Status::Released;
|
||||||
|
SalesHeader.Modify(true);
|
||||||
|
|
||||||
|
OnAfterPostInvoice(SalesHeader);
|
||||||
|
end;
|
||||||
|
|
||||||
|
local procedure CreateCustomerLedgerEntry(var SalesHeader: Record "Sales Header")
|
||||||
|
begin
|
||||||
|
// Posts the customer ledger entry (critical; must never be skipped).
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeCalculateInvoiceDiscount(var SalesHeader: Record "Sales Header"; var DiscountAmount: Decimal; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterPostInvoice(var SalesHeader: Record "Sales Header")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: events
|
||||||
|
keywords: [ishandled, critical-operations, posting, data-integrity, ledger, integration-event, safety]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Do not bypass critical operations with IsHandled
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
The IsHandled override pattern lets a subscriber skip the guarded code entirely. That is acceptable around a pure, side-effect-free calculation, but dangerous around critical operations — posting, ledger-entry creation, number-series consumption, and referential-integrity or permission validation. Wrapping those in `OnBeforeX(…; var IsHandled); if IsHandled then exit;` lets any subscriber silently suppress them, risking imbalanced ledgers, orphaned documents, skipped permission checks, or duplicated numbers — corruption that surfaces far from the subscriber that caused it. Make the calculation overridable, not the commit: expose the value computation through IsHandled, or offer a regular `OnAfter…` event to adjust results, while the critical work runs unconditionally.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Scope IsHandled to a safe value-calculation block and run the critical operations unconditionally afterwards; or expose a positive `OnAfter…` event for subscribers to adjust results, rather than a bypass around the commit.
|
||||||
|
|
||||||
|
See sample: `do-not-bypass-critical-operations-with-ishandled.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
An `OnBefore…` IsHandled guard wrapping a posting or ledger routine — `if IsHandled then exit;` around the code that creates ledger entries and updates document status — letting subscribers skip the commit. Detection: an `if IsHandled then exit;` whose skipped body performs posting, ledger writes, number-series consumption, or integrity and permission validation.
|
||||||
|
|
||||||
|
See sample: `do-not-bypass-critical-operations-with-ishandled.bad.al`.
|
||||||
|
|
@ -0,0 +1,22 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50266 "Loop Event Bad Sample"
|
||||||
|
{
|
||||||
|
procedure ProcessLines(var SalesLine: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
if SalesLine.FindSet() then
|
||||||
|
repeat
|
||||||
|
// Anti-pattern: an event raised on every iteration. Each
|
||||||
|
// subscriber runs once per line, so the cost scales with the
|
||||||
|
// row count and large batches can time out.
|
||||||
|
OnProcessLine(SalesLine);
|
||||||
|
|
||||||
|
SalesLine."Line Amount" := SalesLine.Quantity * SalesLine."Unit Price";
|
||||||
|
SalesLine.Modify(true);
|
||||||
|
until SalesLine.Next() = 0;
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnProcessLine(var SalesLine: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,28 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50265 "Loop Event Good Sample"
|
||||||
|
{
|
||||||
|
procedure ProcessLines(var SalesLine: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
// Fire once before the loop; subscribers act on the whole set.
|
||||||
|
OnBeforeProcessLines(SalesLine);
|
||||||
|
|
||||||
|
if SalesLine.FindSet() then
|
||||||
|
repeat
|
||||||
|
SalesLine."Line Amount" := SalesLine.Quantity * SalesLine."Unit Price";
|
||||||
|
SalesLine.Modify(true);
|
||||||
|
until SalesLine.Next() = 0;
|
||||||
|
|
||||||
|
// Fire once after the loop.
|
||||||
|
OnAfterProcessLines(SalesLine);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeProcessLines(var SalesLine: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterProcessLines(var SalesLine: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: events
|
||||||
|
keywords: [performance, loops, event-publishing, batch, onbefore, onafter, subscriber-cost]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Do not publish events inside loops
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Raising an event on every iteration of a loop multiplies the cost of every subscriber by the number of records. A subscriber doing even a little work per call can turn a fast batch into a timeout when the loop runs over thousands of rows, and the publisher has no control over how expensive a subscriber is. Unless a genuine per-row hook is required, publish once before the loop and once after it, passing enough context — filters, a key, or a buffer — for subscribers to act on the whole set at once. Generated code tends to drop an event inside the `repeat … until` without weighing the per-iteration multiplier.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Raise `OnBeforeProcessLines` before the loop and `OnAfterProcessLines` after it, outside the `repeat … until`, so each subscriber runs once per batch rather than once per row. Give those events the record or filters they need to operate on the whole set.
|
||||||
|
|
||||||
|
See sample: `do-not-publish-events-inside-loops.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
An event raised inside the loop body, fired once per iteration, so subscriber cost scales with the row count and large batches slow down or time out. Detection: an `OnBefore…`/`OnAfter…`/`On…` raise located between `repeat` and `until` in a record loop.
|
||||||
|
|
||||||
|
See sample: `do-not-publish-events-inside-loops.bad.al`.
|
||||||
|
|
@ -0,0 +1,31 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50241 "IsHandled Init Bad Sample"
|
||||||
|
{
|
||||||
|
procedure ApplyDiscounts(var SalesHeader: Record "Sales Header")
|
||||||
|
var
|
||||||
|
DiscountPct: Decimal;
|
||||||
|
IsHandled: Boolean;
|
||||||
|
begin
|
||||||
|
// IsHandled is never initialized before the first raise, so flow depends
|
||||||
|
// on the variable's default rather than an explicit, documented intent.
|
||||||
|
OnBeforeApplyHeaderDiscount(SalesHeader, DiscountPct, IsHandled);
|
||||||
|
if not IsHandled then
|
||||||
|
DiscountPct := 5;
|
||||||
|
|
||||||
|
// Bug: IsHandled is not reset. If the first subscriber set it true, the
|
||||||
|
// payment-discount default below is silently skipped too.
|
||||||
|
OnBeforeApplyPaymentDiscount(SalesHeader, DiscountPct, IsHandled);
|
||||||
|
if not IsHandled then
|
||||||
|
DiscountPct += 2;
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeApplyHeaderDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeApplyPaymentDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,31 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50240 "IsHandled Init Good Sample"
|
||||||
|
{
|
||||||
|
procedure ApplyDiscounts(var SalesHeader: Record "Sales Header")
|
||||||
|
var
|
||||||
|
DiscountPct: Decimal;
|
||||||
|
IsHandled: Boolean;
|
||||||
|
begin
|
||||||
|
IsHandled := false;
|
||||||
|
OnBeforeApplyHeaderDiscount(SalesHeader, DiscountPct, IsHandled);
|
||||||
|
if not IsHandled then
|
||||||
|
DiscountPct := 5;
|
||||||
|
|
||||||
|
// Reset before reusing the same variable for the next event so a
|
||||||
|
// subscriber that handled the first raise can't suppress this one.
|
||||||
|
IsHandled := false;
|
||||||
|
OnBeforeApplyPaymentDiscount(SalesHeader, DiscountPct, IsHandled);
|
||||||
|
if not IsHandled then
|
||||||
|
DiscountPct += 2;
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeApplyHeaderDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeApplyPaymentDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: events
|
||||||
|
keywords: [ishandled, initialization, deterministic, onbefore, reset, integration-event, control-flow]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Initialize IsHandled to false before publishing
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
A routine that raises an `OnBefore…` integration event with a `var IsHandled: Boolean` parameter passes that variable in by reference, so its incoming value decides whether the default logic is skipped. A freshly declared Boolean starts as `false`, but the same variable is frequently reused to raise several events in one routine, and after the first raise it may already be `true`. Assigning `IsHandled := false;` on the line immediately before every raise makes the control flow deterministic and self-documenting, and prevents a stale `true` from silently suppressing logic the author never meant to make skippable. Generated code often reuses one `IsHandled` across several raises without resetting it.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Set `IsHandled := false;` immediately before each `OnBeforeX(…, IsHandled)` raise, then guard the default logic with `if IsHandled then exit;` or `if not IsHandled then …`. Do this even when the variable was just declared: the explicit reset documents intent and stays correct if a second event raise is added to the routine later.
|
||||||
|
|
||||||
|
See sample: `initialize-ishandled-to-false-before-publishing.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Raising `OnBeforeX(…, IsHandled)` with a variable whose value carries over from an earlier raise, so a subscriber that handled the first event unintentionally suppresses the second routine's default logic. Detection: an `IsHandled` variable passed to more than one event in a routine without an intervening `IsHandled := false;`, or any `OnBefore…` raise not preceded by an explicit reset.
|
||||||
|
|
||||||
|
See sample: `initialize-ishandled-to-false-before-publishing.bad.al`.
|
||||||
|
|
@ -0,0 +1,15 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50276 "Param Naming Bad Sample"
|
||||||
|
{
|
||||||
|
procedure RegisterPayment(var SalesHdr: Record "Sales Header"; DocNo: Code[20]; Amt: Decimal)
|
||||||
|
begin
|
||||||
|
// Anti-pattern: abbreviated parameter names force every subscriber to
|
||||||
|
// guess what SalesHdr, DocNo and Amt mean.
|
||||||
|
OnAfterRegisterPayment(SalesHdr, DocNo, Amt);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterRegisterPayment(var SalesHdr: Record "Sales Header"; DocNo: Code[20]; Amt: Decimal)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,14 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50275 "Param Naming Good Sample"
|
||||||
|
{
|
||||||
|
procedure RegisterPayment(var SalesHeader: Record "Sales Header"; DocumentNo: Code[20]; Amount: Decimal)
|
||||||
|
begin
|
||||||
|
// Full, spelled-out names make the event contract self-explanatory.
|
||||||
|
OnAfterRegisterPayment(SalesHeader, DocumentNo, Amount);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterRegisterPayment(var SalesHeader: Record "Sales Header"; DocumentNo: Code[20]; Amount: Decimal)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: events
|
||||||
|
keywords: [parameter-naming, readability, conventions, event-parameters, no-abbreviations, integration-event, clarity]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Name event parameters without abbreviations
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Event parameter names are part of the public contract a subscriber codes against, so they must be self-explanatory. Record parameters take the full table name with the spaces removed — `SalesHeader` for `"Sales Header"`, not `SalesHdr` or `SH`. Simple parameters get a descriptive, spelled-out name — `DocumentNo`, not `DocNo`; `Amount`, not `Amt`. Abbreviated names force every subscriber author to guess intent and tend to be inconsistent across a codebase, where the same concept appears under several contractions. The cost of a clear name is paid once at the publisher; the cost of a cryptic one is paid by every subscriber that has to decode it.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Use full, unabbreviated names: `(SalesHeader: Record "Sales Header"; DocumentNo: Code[20]; Amount: Decimal)`. Record parameters mirror the table name without spaces, and value parameters read as whole words so the contract is unambiguous.
|
||||||
|
|
||||||
|
See sample: `name-event-parameters-without-abbreviations.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Abbreviated parameter names (`SalesHdr`, `DocNo`, `Amt`) that obscure meaning and vary across publishers, so subscribers must guess what each one holds. Detection: event parameters whose names are truncated forms of the table name or contracted words rather than the full term.
|
||||||
|
|
||||||
|
See sample: `name-event-parameters-without-abbreviations.bad.al`.
|
||||||
|
|
@ -0,0 +1,35 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50256 "Event Naming Bad Sample"
|
||||||
|
{
|
||||||
|
procedure PostSalesLine(var SalesLine: Record "Sales Line")
|
||||||
|
var
|
||||||
|
LineAmount: Decimal;
|
||||||
|
begin
|
||||||
|
// Anti-pattern: names don't encode the host routine or the
|
||||||
|
// before/after position, so subscribers can't tell when they fire.
|
||||||
|
BeforePost(SalesLine);
|
||||||
|
|
||||||
|
LineAmount := SalesLine.Quantity * SalesLine."Unit Price";
|
||||||
|
MyCustomSalesEvent(SalesLine, LineAmount);
|
||||||
|
|
||||||
|
SalesLine."Line Amount" := LineAmount;
|
||||||
|
SalesLine.Modify(true);
|
||||||
|
|
||||||
|
SalesLineEvent(SalesLine);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure BeforePost(var SalesLine: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure MyCustomSalesEvent(var SalesLine: Record "Sales Line"; var LineAmount: Decimal)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure SalesLineEvent(var SalesLine: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,36 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50255 "Event Naming Good Sample"
|
||||||
|
{
|
||||||
|
procedure PostSalesLine(var SalesLine: Record "Sales Line")
|
||||||
|
var
|
||||||
|
LineAmount: Decimal;
|
||||||
|
begin
|
||||||
|
// Start of the routine: OnBefore<Name>.
|
||||||
|
OnBeforePostSalesLine(SalesLine);
|
||||||
|
|
||||||
|
LineAmount := SalesLine.Quantity * SalesLine."Unit Price";
|
||||||
|
// Middle of the routine: On<Name>OnAfter<Context>.
|
||||||
|
OnPostSalesLineOnAfterCalcAmounts(SalesLine, LineAmount);
|
||||||
|
|
||||||
|
SalesLine."Line Amount" := LineAmount;
|
||||||
|
SalesLine.Modify(true);
|
||||||
|
|
||||||
|
// End of the routine: OnAfter<Name>.
|
||||||
|
OnAfterPostSalesLine(SalesLine);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforePostSalesLine(var SalesLine: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnPostSalesLineOnAfterCalcAmounts(var SalesLine: Record "Sales Line"; var LineAmount: Decimal)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterPostSalesLine(var SalesLine: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: events
|
||||||
|
keywords: [event-naming, onbefore, onafter, conventions, discoverability, integration-event, publisher]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Name events by publisher position
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
An event name should tell a subscriber where in the publisher the event fires. The convention encodes the position: an event at the very start of a procedure or trigger is `OnBefore<Name>`; one at the very end is `OnAfter<Name>`; one in the middle names both the host routine and the local boundary, as `On<Name>OnBefore<Context>` or `On<Name>OnAfter<Context>`. Consistent, position-encoding names make events discoverable and predictable, and let developers and tooling reason about firing order without reading the publisher. Ad-hoc names such as `MyCustomEvent` or `BeforePost` hide where the event fires and break the conventions the ecosystem relies on.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Name by position: `OnBeforePostSalesLine` and `OnAfterPostSalesLine` at the routine boundaries, and `OnPostSalesLineOnAfterCalcAmounts` for an event raised partway through `PostSalesLine` after an amount calculation. The name alone then tells a subscriber both the host routine and the exact point it runs.
|
||||||
|
|
||||||
|
See sample: `name-events-by-publisher-position.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Ad-hoc event names that omit the host routine or the before/after position (`MyCustomSalesEvent`, `BeforePost`, `SalesLineEvent`), leaving subscribers unable to tell when the event fires relative to the publisher's logic. Detection: publisher names that do not follow the `OnBefore`/`OnAfter<Routine>` or `On<Routine>OnBefore`/`OnAfter<Context>` patterns.
|
||||||
|
|
||||||
|
See sample: `name-events-by-publisher-position.bad.al`.
|
||||||
|
|
@ -0,0 +1,27 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50261 "Reuse Event Bad Sample"
|
||||||
|
{
|
||||||
|
procedure ProcessOrder(var SalesHeader: Record "Sales Header"; CustomerNo: Code[20])
|
||||||
|
var
|
||||||
|
IsHandled: Boolean;
|
||||||
|
begin
|
||||||
|
IsHandled := false;
|
||||||
|
// Anti-pattern: a near-duplicate event raised right next to the original,
|
||||||
|
// differing only by an extra parameter — two consecutive events where a
|
||||||
|
// single extended event would do.
|
||||||
|
OnBeforeProcessOrder(SalesHeader, IsHandled);
|
||||||
|
OnBeforeProcessOrderWithCustomer(SalesHeader, CustomerNo, IsHandled);
|
||||||
|
if IsHandled then
|
||||||
|
exit;
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeProcessOrder(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeProcessOrderWithCustomer(var SalesHeader: Record "Sales Header"; CustomerNo: Code[20]; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,20 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50260 "Reuse Event Good Sample"
|
||||||
|
{
|
||||||
|
procedure ProcessOrder(var SalesHeader: Record "Sales Header"; CustomerNo: Code[20])
|
||||||
|
var
|
||||||
|
IsHandled: Boolean;
|
||||||
|
begin
|
||||||
|
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);
|
||||||
|
if IsHandled then
|
||||||
|
exit;
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeProcessOrder(var SalesHeader: Record "Sales Header"; CustomerNo: Code[20]; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: events
|
||||||
|
keywords: [event-reuse, duplication, consecutive-events, extension-point, onbefore, integration-event, maintainability]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Prefer reusing or extending existing events
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Before adding a publisher, check whether an event already fires at that point in the code. Two related smells signal that you should reuse or extend instead of adding one. The first is a brand-new event placed directly next to an existing one — two consecutive event raises with no logic between them, which gives subscribers two seams where one belongs. The second is a near-duplicate event that differs from an existing one only by an extra parameter. Both bloat the publisher surface and leave subscribers unsure which event to pick. Prefer subscribing to the existing event, or extending it by appending the parameter you need, over introducing a parallel one.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
When the data you need is already exposed at an existing event, subscribe to it. When the event lacks a parameter, extend that event by appending the parameter at the end — one publisher, one raise — rather than adding a second event beside it.
|
||||||
|
|
||||||
|
See sample: `prefer-reusing-or-extending-existing-events.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Adding a second event raise immediately after an existing one, or creating `OnBeforeProcessOrderWithCustomer` next to `OnBeforeProcessOrder` just to add a single parameter. Detection: two consecutive `OnBefore…`/`OnAfter…` raises with no logic between them, or near-duplicate event names differing only by a parameter-describing suffix.
|
||||||
|
|
||||||
|
See sample: `prefer-reusing-or-extending-existing-events.bad.al`.
|
||||||
|
|
@ -0,0 +1,15 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50281 "Sender This Bad Sample"
|
||||||
|
{
|
||||||
|
procedure ProcessOrder(OrderNo: Code[20])
|
||||||
|
begin
|
||||||
|
OnBeforeProcessOrder(OrderNo);
|
||||||
|
end;
|
||||||
|
|
||||||
|
// Anti-pattern: IncludeSender = true is used only to expose the publisher
|
||||||
|
// instance to subscribers; a codeunit can pass 'this' explicitly instead.
|
||||||
|
[IntegrationEvent(true, false)]
|
||||||
|
local procedure OnBeforeProcessOrder(OrderNo: Code[20])
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,14 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50280 "Sender This Good Sample"
|
||||||
|
{
|
||||||
|
procedure ProcessOrder(OrderNo: Code[20])
|
||||||
|
begin
|
||||||
|
// Pass the current instance explicitly as a typed Sender parameter.
|
||||||
|
OnBeforeProcessOrder(OrderNo, this);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeProcessOrder(OrderNo: Code[20]; Sender: Codeunit "Sender This Good Sample")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [25..]
|
||||||
|
domain: events
|
||||||
|
keywords: [this-keyword, includesender, sender, codeunit, self-reference, integration-event, type-safety]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Prefer this over IncludeSender in 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.
|
||||||
|
|
||||||
|
## 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.
|
||||||
|
|
||||||
|
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.
|
||||||
|
|
||||||
|
See sample: `prefer-this-over-includesender-in-codeunit-events.bad.al`.
|
||||||
|
|
@ -0,0 +1,16 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50271 "Temp Param Bad Sample"
|
||||||
|
{
|
||||||
|
procedure SummarizeLines(var SalesLineBuffer: Record "Sales Line" temporary)
|
||||||
|
begin
|
||||||
|
// Anti-pattern: the parameter is temporary but isn't named with a Temp
|
||||||
|
// prefix, so subscribers can't tell the data isn't persisted and may
|
||||||
|
// rely on writes that are discarded.
|
||||||
|
OnAfterSummarizeLines(SalesLineBuffer);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterSummarizeLines(var SalesLineBuffer: Record "Sales Line" temporary)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,14 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50270 "Temp Param Good Sample"
|
||||||
|
{
|
||||||
|
procedure SummarizeLines(var TempSalesLineBuffer: Record "Sales Line" temporary)
|
||||||
|
begin
|
||||||
|
// The Temp prefix tells subscribers the buffer isn't persisted.
|
||||||
|
OnAfterSummarizeLines(TempSalesLineBuffer);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterSummarizeLines(var TempSalesLineBuffer: Record "Sales Line" temporary)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: events
|
||||||
|
keywords: [temporary-record, naming, event-parameters, buffer, temp-prefix, integration-event, conventions]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Prefix temporary record event parameters with Temp
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
When a record passed to an event is a temporary record — an in-memory buffer not persisted to the database — its parameter name must start with `Temp`. The prefix is the only reliable signal a subscriber has that writes to the record will not reach the database and that the data is scoped to the current call. Without it, subscribers may treat buffer data as persisted: calling `Modify` or `Insert` expecting durability, or reading it as the authoritative table, which leads to silent data loss and confusing behaviour. The `temporary` keyword sits on the variable declaration and is not visible at the subscriber, so the name has to carry the meaning.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Name temporary record parameters with a `Temp` prefix, for example `var TempSalesLineBuffer: Record "Sales Line" temporary`, so every subscriber sees immediately that the record is an in-memory buffer and treats writes accordingly.
|
||||||
|
|
||||||
|
See sample: `prefix-temporary-record-event-parameters-with-temp.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
A temporary record parameter named without the `Temp` prefix (`var SalesLineBuffer: Record "Sales Line" temporary`), so subscribers cannot tell the record is non-persistent and may rely on writes that are silently discarded. Detection: an event parameter declared `temporary` whose name does not start with `Temp`.
|
||||||
|
|
||||||
|
See sample: `prefix-temporary-record-event-parameters-with-temp.bad.al`.
|
||||||
|
|
@ -0,0 +1,32 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50246 "OnAfter Preserve Bad Sample"
|
||||||
|
{
|
||||||
|
procedure ReleaseDocument(var SalesHeader: Record "Sales Header")
|
||||||
|
var
|
||||||
|
IsHandled: Boolean;
|
||||||
|
begin
|
||||||
|
IsHandled := false;
|
||||||
|
OnBeforeReleaseDocument(SalesHeader, IsHandled);
|
||||||
|
|
||||||
|
// Bug: returning here also skips OnAfterReleaseDocument below, so
|
||||||
|
// subscribers that rely on the after-event stop running whenever
|
||||||
|
// another extension handles the OnBefore.
|
||||||
|
if IsHandled then
|
||||||
|
exit;
|
||||||
|
|
||||||
|
SalesHeader.Status := SalesHeader.Status::Released;
|
||||||
|
SalesHeader.Modify(true);
|
||||||
|
|
||||||
|
OnAfterReleaseDocument(SalesHeader);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeReleaseDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterReleaseDocument(var SalesHeader: Record "Sales Header")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,30 @@
|
||||||
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
|
codeunit 50245 "OnAfter Preserve Good Sample"
|
||||||
|
{
|
||||||
|
procedure ReleaseDocument(var SalesHeader: Record "Sales Header")
|
||||||
|
var
|
||||||
|
IsHandled: Boolean;
|
||||||
|
begin
|
||||||
|
IsHandled := false;
|
||||||
|
OnBeforeReleaseDocument(SalesHeader, IsHandled);
|
||||||
|
|
||||||
|
// Skip only the default body, not the routine, so OnAfter still fires.
|
||||||
|
if not IsHandled then begin
|
||||||
|
SalesHeader.Status := SalesHeader.Status::Released;
|
||||||
|
SalesHeader.Modify(true);
|
||||||
|
end;
|
||||||
|
|
||||||
|
// Fires whether or not a subscriber handled the body above.
|
||||||
|
OnAfterReleaseDocument(SalesHeader);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnBeforeReleaseDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnAfterReleaseDocument(var SalesHeader: Record "Sales Header")
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: events
|
||||||
|
keywords: [ishandled, onafter, event-pairing, control-flow, guard, integration-event, side-effects]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Preserve OnAfter execution when IsHandled skips the body
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
A routine that exposes both an `OnBefore…` event (with `var IsHandled`) and a paired `OnAfter…` event has a subtle trap. The common `if IsHandled then exit;` guard returns from the whole routine, so when a subscriber handles the OnBefore the OnAfter event never fires. Subscribers that depend on OnAfter — logging, downstream integration, dependent updates — then silently stop running whenever some other extension overrides the body. The fix is to skip only the default body, not the routine, so the OnAfter still publishes. The two seams are independent: overriding the work should not cancel the notification that the work happened.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Wrap only the default work in `if not IsHandled then begin … end;` and keep the `OnAfterX(…)` raise after that block, outside the guard, so it always fires regardless of whether a subscriber handled the OnBefore. This keeps the override seam and the after-notification independent, which is what subscribers expect.
|
||||||
|
|
||||||
|
See sample: `preserve-onafter-execution-when-ishandled-skips-the-body.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Guarding with `if IsHandled then exit;` and placing the `OnAfterX` raise later in the same routine, so handling the OnBefore short-circuits the whole procedure and the OnAfter event is skipped along with the body. Detection: an `if IsHandled then exit;` in a routine that also raises a paired `OnAfter…` event after that point.
|
||||||
|
|
||||||
|
See sample: `preserve-onafter-execution-when-ishandled-skips-the-body.bad.al`.
|
||||||
|
|
@ -39,7 +39,7 @@ Narrow the relevant files to the subset that applies to the changes under review
|
||||||
|
|
||||||
- The changed AL object names and types — especially codeunits that publish events or host event subscribers, posting/release/validation routines that should expose extension points, and test codeunits that bind subscribers.
|
- The changed AL object names and types — especially codeunits that publish events or host event subscribers, posting/release/validation routines that should expose extension points, and test codeunits that bind subscribers.
|
||||||
- The changed procedures and triggers, weighted toward event publisher methods, methods carrying the `[EventSubscriber(...)]` attribute, routines that raise `OnBefore`/`OnAfter` events, and any procedure that calls `BindSubscription`/`UnbindSubscription`.
|
- The changed procedures and triggers, weighted toward event publisher methods, methods carrying the `[EventSubscriber(...)]` attribute, routines that raise `OnBefore`/`OnAfter` events, and any procedure that calls `BindSubscription`/`UnbindSubscription`.
|
||||||
- Tokens extracted from the diff that relate to events and the publish/subscribe model (`IntegrationEvent`, `BusinessEvent`, `EventSubscriber`, `IsHandled`, `BindSubscription`, `UnbindSubscription`, `EventSubscriberInstance`, `OnBefore`, `OnAfter`, `Manual`).
|
- Tokens extracted from the diff that relate to events and the publish/subscribe model (`IntegrationEvent`, `BusinessEvent`, `EventSubscriber`, `IsHandled`, `BindSubscription`, `UnbindSubscription`, `EventSubscriberInstance`, `OnBefore`, `OnAfter`, `Manual`, `IncludeSender`, `Sender`, `this`, `RecordRef`, `xRec`, `temporary`, `Temp`, `repeat`).
|
||||||
|
|
||||||
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
|
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
|
||||||
|
|
||||||
|
|
@ -47,6 +47,23 @@ Once the candidate worklist is known, resolve layer-precedence conflicts per REA
|
||||||
|
|
||||||
When the post-conflict worklist is empty because no applicable events knowledge exists, or because configuration suppressed every candidate, emit `outcome: "no-knowledge"`. When the worklist is empty because no applicable events knowledge matched the changes, emit `outcome: "completed"` with an empty `findings` array.
|
When the post-conflict worklist is empty because no applicable events knowledge exists, or because configuration suppressed every candidate, emit `outcome: "no-knowledge"`. When the worklist is empty because no applicable events knowledge matched the changes, emit `outcome: "completed"` with an empty `findings` array.
|
||||||
|
|
||||||
|
### Event-design checks
|
||||||
|
|
||||||
|
The following targeted checks map diff signals to specific `events` articles. Treat each as a candidate-selection cue: when the signal appears in the changed code, add the named article to the worklist and evaluate it in Action.
|
||||||
|
|
||||||
|
- `IsHandled` raised without an immediately preceding `IsHandled := false;`, or one `IsHandled` variable reused across several raises with no reset between them — `initialize-ishandled-to-false-before-publishing`.
|
||||||
|
- `if IsHandled then exit;` in a routine that also raises a paired `OnAfter…` event later, so the after-event is skipped whenever the call is handled — `preserve-onafter-execution-when-ishandled-skips-the-body`.
|
||||||
|
- A parameter added before existing parameters on a changed event signature instead of appended at the end — `add-new-event-parameters-at-the-end`.
|
||||||
|
- Publisher names that do not encode firing position (`OnBefore`/`OnAfter<Routine>` at the boundaries, `On<Routine>OnBefore`/`OnAfter<Context>` mid-routine) — `name-events-by-publisher-position`.
|
||||||
|
- Two consecutive `OnBefore`/`OnAfter` raises with no logic between them, or a near-duplicate event differing only by an extra parameter — `prefer-reusing-or-extending-existing-events`.
|
||||||
|
- An event raised between `repeat` and `until` inside a record loop — `do-not-publish-events-inside-loops`.
|
||||||
|
- A `temporary` record event parameter whose name does not start with `Temp` — `prefix-temporary-record-event-parameters-with-temp`.
|
||||||
|
- Abbreviated event parameter names (`SalesHdr`, `DocNo`, `Amt`) instead of full table names and spelled-out values — `name-event-parameters-without-abbreviations`.
|
||||||
|
- `[IntegrationEvent(true, …)]` (`IncludeSender`) on a codeunit event used only to expose the publisher, where `this` could be passed as a typed `Sender` parameter (Business Central 2024 release wave 2 and later) — `prefer-this-over-includesender-in-codeunit-events`.
|
||||||
|
- A `RecordRef` event parameter, or a passed-through `xRec`, where a concrete typed record fits — `avoid-loosely-typed-event-parameters`.
|
||||||
|
- A `var IsHandled` added to a pre-existing event rather than introduced through a new `OnBefore` publisher — `do-not-add-ishandled-to-an-existing-event`.
|
||||||
|
- An `if IsHandled then exit;` whose skipped body performs posting, ledger-entry creation, number-series consumption, or integrity/permission validation — `do-not-bypass-critical-operations-with-ishandled`.
|
||||||
|
|
||||||
## Action
|
## Action
|
||||||
|
|
||||||
For each worklist entry, evaluate the diff against the file's `## Best Practice` and `## Anti Pattern` sections. Emit findings as follows:
|
For each worklist entry, evaluate the diff against the file's `## Best Practice` and `## Anti Pattern` sections. Emit findings as follows:
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue