diff --git a/community/knowledge/architecture/defensive-vs-offensive-code-must-match-blast-radius.bad.al b/community/knowledge/architecture/defensive-vs-offensive-code-must-match-blast-radius.bad.al new file mode 100644 index 0000000..b6812b2 --- /dev/null +++ b/community/knowledge/architecture/defensive-vs-offensive-code-must-match-blast-radius.bad.al @@ -0,0 +1,14 @@ +codeunit 50150 "Contoso Header Field Lookup" +{ + procedure GetVatProdPostingGroup(DocumentNo: Code[20]): Code[20] + var + Header: Record "Contoso Document Header"; + begin + // The same guarded shape used for a low-risk field is reused here + // without re-examining the consequence — a missing header now + // silently posts with a blank VAT posting group. + if Header.Get(DocumentNo) then + exit(Header."VAT Prod. Posting Group"); + exit(''); + end; +} diff --git a/community/knowledge/architecture/defensive-vs-offensive-code-must-match-blast-radius.good.al b/community/knowledge/architecture/defensive-vs-offensive-code-must-match-blast-radius.good.al new file mode 100644 index 0000000..d922a11 --- /dev/null +++ b/community/knowledge/architecture/defensive-vs-offensive-code-must-match-blast-radius.good.al @@ -0,0 +1,22 @@ +codeunit 50150 "Contoso Header Field Lookup" +{ + procedure GetVatRegNo(DocumentNo: Code[20]): Text[20] + var + Header: Record "Contoso Document Header"; + begin + // Low blast radius: informational field, safe to default if missing. + if Header.Get(DocumentNo) then + exit(Header."VAT Registration No."); + exit(''); + end; + + procedure GetVatProdPostingGroup(DocumentNo: Code[20]): Code[20] + var + Header: Record "Contoso Document Header"; + begin + // High blast radius: feeds a posted VAT calculation. Fail loud. + Header.Get(DocumentNo); + Header.TestField("VAT Prod. Posting Group"); + exit(Header."VAT Prod. Posting Group"); + end; +} diff --git a/community/knowledge/architecture/defensive-vs-offensive-code-must-match-blast-radius.md b/community/knowledge/architecture/defensive-vs-offensive-code-must-match-blast-radius.md new file mode 100644 index 0000000..ddc8072 --- /dev/null +++ b/community/knowledge/architecture/defensive-vs-offensive-code-must-match-blast-radius.md @@ -0,0 +1,20 @@ +--- +bc-version: [all] +domain: architecture +keywords: [defensive-programming, offensive-programming, fail-fast, blast-radius, guarded-lookup, error-handling] +technologies: [al] +countries: [w1] +application-area: [all] +--- +# Defensive vs. offensive code must match the blast radius of being wrong + +> Contributions welcome — open a PR to refine or extend this article. + +## Description +Whether a piece of code should guard gracefully (defensive) or fail loudly (offensive/fail-fast) depends on what happens downstream if the guarded condition is silently defaulted or skipped — it isn't a matter of habit or a blanket house style. Treating every missing value the same way, defensively or offensively, is itself the anti-pattern: uniform defensiveness hides the failures that matter most, and uniform fail-fast turns ordinary, expected absence into unnecessary crashes. Two fields can look structurally identical — both read from a related record, both potentially missing — and still deserve opposite treatment: a VAT registration number used only on a printed document is something a reviewer will likely catch before the document ships, but a VAT posting group that determines the tax applied to a posted transaction is not — once posted, it's wrong money on a ledger entry, discoverable only by someone specifically auditing VAT postings. + +## Best Practice +Trace what a silently-defaulted or skipped value actually reaches before choosing how to guard it. If it reaches a posted ledger amount, a tax calculation, a quantity or price actually used in a transaction, or a compliance-facing output, code offensively: let the lookup fail loud (`TestField`, an unguarded `Get()` expected to always succeed, or an explicit `Error`) so a human sees the problem before anything posts. If it's cosmetic, informational, or easily corrected after the fact, code defensively — guard the lookup, but choose the fallback deliberately rather than accepting whatever the datatype's zero-value default happens to be. + +## Anti Pattern +Guarding two fields the same way "out of habit" when they carry different blast radii — for example, defaulting both a VAT registration number and a VAT posting group to blank on a failed lookup — treats a cosmetic field and a transaction-critical one as equally safe to silently default. A reviewer can spot this by a guarded `Get()` feeding a value that reaches a posted amount, a tax calculation, or a compliance output, with no corresponding `TestField` or explicit error on the missing-value path. diff --git a/community/knowledge/architecture/posting-routines-must-follow-check-post-line-batch-pattern.bad.al b/community/knowledge/architecture/posting-routines-must-follow-check-post-line-batch-pattern.bad.al new file mode 100644 index 0000000..ff678fe --- /dev/null +++ b/community/knowledge/architecture/posting-routines-must-follow-check-post-line-batch-pattern.bad.al @@ -0,0 +1,48 @@ +codeunit 50122 "Contoso Jnl.-Post Batch" +{ + // Check and Post responsibilities are conflated into Post Batch, and + // Post Line reads the Journal table directly — it can no longer be + // called on its own by a document posting routine. + procedure PostBatch(var JnlLine: Record "Contoso Journal Line") + begin + if JnlLine.FindSet() then + repeat + JnlLine.TestField("Posting Date"); // Check Line's job + PostLine(JnlLine); + until JnlLine.Next() = 0; + end; + + local procedure PostLine(var JnlLine: Record "Contoso Journal Line") + var + LedgerEntry: Record "Contoso Ledger Entry"; + begin + LedgerEntry.Init(); + LedgerEntry.TransferFields(JnlLine); + LedgerEntry.Insert(true); + JnlLine.Delete(); // touches the Journal table — no longer reusable elsewhere + end; +} + +page 50120 "Contoso Journal" +{ + PageType = Worksheet; + SourceTable = "Contoso Journal Line"; + + actions + { + area(Processing) + { + action(Post) + { + trigger OnAction() + var + PostBatch: Codeunit "Contoso Jnl.-Post Batch"; + begin + // Calls -Post Batch directly — no confirmation wrapper, + // and this codeunit can never be driven unattended. + PostBatch.PostBatch(Rec); + end; + } + } + } +} diff --git a/community/knowledge/architecture/posting-routines-must-follow-check-post-line-batch-pattern.good.al b/community/knowledge/architecture/posting-routines-must-follow-check-post-line-batch-pattern.good.al new file mode 100644 index 0000000..5a4bcdd --- /dev/null +++ b/community/knowledge/architecture/posting-routines-must-follow-check-post-line-batch-pattern.good.al @@ -0,0 +1,59 @@ +codeunit 50120 "Contoso Jnl.-Check Line" +{ + procedure CheckLine(var JnlLine: Record "Contoso Journal Line") + begin + if JnlLine."Line No." = 0 then + exit; // skip empty lines without error + + JnlLine.TestField("Posting Date"); + JnlLine.TestField(Amount); + end; +} + +codeunit 50121 "Contoso Jnl.-Post Line" +{ + // Operates only on the record passed in — never reads or writes the + // Journal table itself, so document posting can call this directly. + procedure PostLine(var JnlLine: Record "Contoso Journal Line") + var + LedgerEntry: Record "Contoso Ledger Entry"; + begin + LedgerEntry.Init(); + LedgerEntry.TransferFields(JnlLine); + LedgerEntry.Insert(true); + end; +} + +codeunit 50122 "Contoso Jnl.-Post Batch" +{ + // The only one of the three that touches the Journal table. + procedure PostBatch(var JnlLine: Record "Contoso Journal Line") + var + CheckLine: Codeunit "Contoso Jnl.-Check Line"; + PostLine: Codeunit "Contoso Jnl.-Post Line"; + begin + if JnlLine.FindSet() then + repeat + CheckLine.CheckLine(JnlLine); + until JnlLine.Next() = 0; + + if JnlLine.FindSet() then + repeat + PostLine.PostLine(JnlLine); + JnlLine.Delete(); + until JnlLine.Next() = 0; + end; +} + +codeunit 50123 "Contoso Jnl.-Post (Yes/No)" +{ + // The only entry point a page should call. + trigger OnRun() + var + JnlLine: Record "Contoso Journal Line"; + PostBatch: Codeunit "Contoso Jnl.-Post Batch"; + begin + if Confirm('Do you want to post the journal lines?') then + PostBatch.PostBatch(JnlLine); + end; +} diff --git a/community/knowledge/architecture/posting-routines-must-follow-check-post-line-batch-pattern.md b/community/knowledge/architecture/posting-routines-must-follow-check-post-line-batch-pattern.md new file mode 100644 index 0000000..86ed190 --- /dev/null +++ b/community/knowledge/architecture/posting-routines-must-follow-check-post-line-batch-pattern.md @@ -0,0 +1,20 @@ +--- +bc-version: [all] +domain: architecture +keywords: [posting-routine, check-line, post-line, post-batch, companion-codeunit, sales-post, yes-no-wrapper, journal] +technologies: [al] +countries: [w1] +application-area: [all] +--- +# Posting routines must follow the Check Line / Post Line / Post Batch pattern + +> Contributions welcome — open a PR to refine or extend this article. + +## Description +Every journal-based posting routine in Business Central is split across three companion codeunits with distinct, non-overlapping responsibilities: ` Check Line` validates one line and shows no UI beyond errors; ` Post Line` writes exactly one line to the ledger/register tables and never touches the Journal table itself, which is what lets other posting code call it directly; ` Post Batch` is the only one of the three that reads and updates the Journal table, looping Check Line then Post Line across all lines. A document posting routine (such as `Sales-Post`) calls `Post Line` directly per document and bypasses `Post Batch` entirely. A new posting routine that conflates these responsibilities, or a page that calls a `-Post` codeunit directly instead of through its `-Post (Yes/No)` confirmation wrapper, breaks assumptions other extensions and unattended batch-posting reports rely on. + +## Best Practice +Keep `Check Line` free of side effects beyond validation and a first-call-only re-read of setup data; keep `Post Line` operating purely on the record variable it's given, never on the Journal table, so other posting code can call it without fabricating a journal line first; let only `Post Batch` read and write the Journal table. Route every page-initiated post through a `-Post (Yes/No)` wrapper that confirms with the user and then calls the interaction-free `-Post` codeunit, so the same posting logic stays safely callable from an unattended batch report. + +## Anti Pattern +A `Post Line` codeunit that reads back from the Journal table, or a `Check Line` codeunit that writes to the database on every call, has taken on its neighbor's responsibility and can no longer be reused safely by other posting code. A page wired directly to a `-Post` codeunit — skipping the `-Post (Yes/No)` wrapper — either shows no confirmation to the user, or, if confirmation is bolted onto the core codeunit instead, makes that codeunit impossible to call from an unattended process. diff --git a/community/knowledge/architecture/prefer-email-module-over-codeunit-mail-397.bad.al b/community/knowledge/architecture/prefer-email-module-over-codeunit-mail-397.bad.al new file mode 100644 index 0000000..9ab57e7 --- /dev/null +++ b/community/knowledge/architecture/prefer-email-module-over-codeunit-mail-397.bad.al @@ -0,0 +1,13 @@ +codeunit 50130 "Contoso Send Confirmation" +{ + procedure SendOrderConfirmation(FromName: Text; ToAddress: Text; Subject: Text; Body: Text) + var + Mail: Codeunit Mail; + begin + // Hard-coded to whatever SMTP account is configured; no Sent/Outbox + // record is left once this call returns. + Mail.CreateMessage(FromName, ToAddress, '', Subject, Body, true); + if not Mail.Send() then + Message(Mail.GetErrorDesc()); + end; +} diff --git a/community/knowledge/architecture/prefer-email-module-over-codeunit-mail-397.good.al b/community/knowledge/architecture/prefer-email-module-over-codeunit-mail-397.good.al new file mode 100644 index 0000000..b36b167 --- /dev/null +++ b/community/knowledge/architecture/prefer-email-module-over-codeunit-mail-397.good.al @@ -0,0 +1,11 @@ +codeunit 50130 "Contoso Send Confirmation" +{ + procedure SendOrderConfirmation(ToAddress: Text; Subject: Text; Body: Text) + var + EmailMessage: Codeunit "Email Message"; + Email: Codeunit Email; + begin + EmailMessage.Create(ToAddress, Subject, Body, true); + Email.Send(EmailMessage, Enum::"Email Scenario"::"Sales Order"); + end; +} diff --git a/community/knowledge/architecture/prefer-email-module-over-codeunit-mail-397.md b/community/knowledge/architecture/prefer-email-module-over-codeunit-mail-397.md new file mode 100644 index 0000000..b5dee56 --- /dev/null +++ b/community/knowledge/architecture/prefer-email-module-over-codeunit-mail-397.md @@ -0,0 +1,20 @@ +--- +bc-version: [all] +domain: architecture +keywords: [email, codeunit-mail, email-message, email-scenario, email-account, smtp, sending-email] +technologies: [al] +countries: [w1] +application-area: [all] +--- +# New email-sending code should use the Email module, not Codeunit Mail (397) + +> Contributions welcome — open a PR to refine or extend this article. + +## Description +Business Central's current extensibility model for sending email is `Codeunit Email`, table `Email Message`, `enum Email Scenario`, and the `Email Account`/`Email Connector` interface — documented under "Extend Email Capabilities" and "Set up email." Older AL code, and some LLM training data, still reaches for the earlier `Codeunit Mail (397)` (`CreateMessage`/`Send`/`GetErrorDesc`), which is hard-coupled to one SMTP-style connector and leaves no record behind once a message is sent. New AL development that sends email should build on the Email module, not `Codeunit Mail (397)`. + +## Best Practice +Create the message through `Codeunit "Email Message"` and send it through `Codeunit Email`, routed by an `Email Scenario` rather than a hard-coded account. This keeps the calling code independent of which connector (Microsoft 365, Current User, SMTP, or a partner connector) the customer has configured, gives every message a tracked Sent/Outbox/Draft status, and lets different document types route through different accounts without the caller needing to know which one. + +## Anti Pattern +Calling `Codeunit Mail`'s `CreateMessage`/`Send`/`GetErrorDesc` in new code still compiles and runs, but it inherits SMTP-era assumptions, leaves no Sent/Outbox trail, and hard-codes the connector choice into the calling code. A reviewer can spot it by any new reference to `Codeunit Mail (397)` outside of already-existing legacy call sites. diff --git a/community/knowledge/performance/document-reports-should-default-to-word-layout.bad.al b/community/knowledge/performance/document-reports-should-default-to-word-layout.bad.al new file mode 100644 index 0000000..e06e22a --- /dev/null +++ b/community/knowledge/performance/document-reports-should-default-to-word-layout.bad.al @@ -0,0 +1,15 @@ +report 50140 "Contoso Sales Quote Confirmation" +{ + // Document report defaulted to RDLC out of habit — inherits the + // sandboxed-app-domain cost for no reason tied to this report's content. + DefaultRenderingLayout = RDLC; + + rendering + { + layout(RDLC) + { + Type = RDLC; + LayoutFile = './Layouts/SalesQuoteConfirmation.rdl'; + } + } +} diff --git a/community/knowledge/performance/document-reports-should-default-to-word-layout.good.al b/community/knowledge/performance/document-reports-should-default-to-word-layout.good.al new file mode 100644 index 0000000..443c846 --- /dev/null +++ b/community/knowledge/performance/document-reports-should-default-to-word-layout.good.al @@ -0,0 +1,14 @@ +report 50140 "Contoso Sales Quote Confirmation" +{ + DefaultRenderingLayout = Word; + + rendering + { + layout(Word) + { + Type = Word; + LayoutFile = './Layouts/SalesQuoteConfirmation.docx'; + Caption = 'Word Layout'; + } + } +} diff --git a/community/knowledge/performance/document-reports-should-default-to-word-layout.md b/community/knowledge/performance/document-reports-should-default-to-word-layout.md new file mode 100644 index 0000000..0a294bd --- /dev/null +++ b/community/knowledge/performance/document-reports-should-default-to-word-layout.md @@ -0,0 +1,20 @@ +--- +bc-version: [all] +domain: performance +keywords: [report-layout, word-layout, rdlc, document-report, sandbox-app-domain, rendering] +technologies: [al] +countries: [w1] +application-area: [all] +--- +# Document reports should default to a Word layout, not RDLC + +> Contributions welcome — open a PR to refine or extend this article. + +## Description +A document report — an invoice, statement, order confirmation, or any report meant to be printed, emailed, or exported as a single-record document — should default to a `Word` rendering layout rather than `RDLC`. This is Microsoft's own documented recommendation, not a style preference: RDLC layouts run in a sandboxed app domain that only lives for the current report invocation, which makes UI-adjacent actions like emailing the resulting document slower than the equivalent Word layout, which isn't subject to that sandbox constraint. Tabular or list reports with heavy aggregation are a separate case and often still fit RDLC or Excel better — the deciding question is whether the report is one structured document per record, not "RDLC vs. Word" as a blanket choice. + +## Best Practice +Set `DefaultRenderingLayout = Word` on a document report and provide a `.docx` layout file, reserving RDLC for reports whose output is a data listing rather than a per-record document — the shape Word's table-based layout model handles poorly. + +## Anti Pattern +Defaulting a new document report to RDLC because it's the more familiar tool, or because a copied template happened to use it, inherits RDLC's sandboxed-app-domain performance cost for a report shape that gains nothing from it. A reviewer can spot this by a document-style report (single record, Header/Lines, meant for printing or emailing) whose `DefaultRenderingLayout` is `RDLC` with no data-listing/aggregation reason for the choice.