mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 06:36:55 +01:00
Fix remaining READ-convention sample links across this PR's 18 articles
The same plain-backtick "See sample: \`x.good.al\`." form fixed on al-methods-limited-during-write-transactions (PR #161) turned up repo-wide on 15 more of this PR's articles - Knowledge-Retrieval.ps1 requires the markdown-link form to associate a sample with its article. All 16 fixed; the four local validators (frontmatter, knowledge-index, knowledge-retrieval, review-fixtures, skill-index) pass.
This commit is contained in:
parent
faa0bceb86
commit
b983a6da57
16 changed files with 32 additions and 32 deletions
|
|
@ -19,10 +19,10 @@ The classic `File` variable type — `Open`/`Create`/`Read`/`Write`/`Close` agai
|
|||
|
||||
Use the stream-based equivalents: `UploadIntoStream` to read user-selected file content into an `InStream`, and `DownloadFromStream` to write an `OutStream`'s content to a file the user saves. Stage the content in a `TempBlob` between the stream and the rest of the parsing/formatting code.
|
||||
|
||||
See sample: `file-datatype-saas.good.al`.
|
||||
See sample: [`file-datatype-saas.good.al`](file-datatype-saas.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Opening a hardcoded or user-supplied filesystem path with the `File` variable type. This is a strong signal the code was written for on-premises only, or copied from material that predates the cloud-first streaming APIs.
|
||||
|
||||
See sample: `file-datatype-saas.bad.al`.
|
||||
See sample: [`file-datatype-saas.bad.al`](file-datatype-saas.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ Older AL code sends email by calling `Codeunit Mail (397)`. Business Central's c
|
|||
|
||||
Build on `Codeunit Email` and `Codeunit "Email Message"`. Route the message through an `Email Scenario` so different document types can use different accounts without the calling code needing to know which account that is, and get a tracked Sent/Outbox/Draft record for free.
|
||||
|
||||
See sample: `prefer-email-module.good.al`.
|
||||
See sample: [`prefer-email-module.good.al`](prefer-email-module.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `Codeunit Mail`'s `CreateMessage`. It still compiles and runs, but current `Codeunit Mail`'s own implementation of `CreateMessage` no longer sends anything by itself — it only raises integration events for a legacy subscriber to act on — so building new code on it means depending on whatever compatibility shim happens to still be wired up, with no first-class connector selection and no queryable Sent/Outbox/Draft record. `Send` and `GetErrorDesc` are not current members of `Codeunit Mail` at all; do not reference them.
|
||||
|
||||
See sample: `prefer-email-module.bad.al`.
|
||||
See sample: [`prefer-email-module.bad.al`](prefer-email-module.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ Business Central's own journal-based posting routines consistently follow a thre
|
|||
|
||||
`Check Line` reads setup/dimension data only on its first call and shows no UI beyond errors. `Post Line` only operates on the record passed to it — never the Journal table — so it can be called directly by other posting code, including a document posting routine. `Post Batch` is the only one of the three that reads and updates the Journal table, and it is the only one invoked from the Post action on a journal page. A `-Post` document codeunit is never called directly from a page; a page calls a `-Post (Yes/No)` confirmation wrapper instead, so the same `-Post` codeunit can also run unattended from a batch-posting report.
|
||||
|
||||
See sample: `check-post-line-batch-pattern.good.al`.
|
||||
See sample: [`check-post-line-batch-pattern.good.al`](check-post-line-batch-pattern.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A single monolithic posting codeunit that reads the Journal table, validates lines, writes ledger entries, and shows confirmation dialogs all in one procedure. It cannot be reused by another posting routine without fabricating journal records, and it cannot run unattended because it insists on user interaction.
|
||||
|
||||
See sample: `check-post-line-batch-pattern.bad.al`.
|
||||
See sample: [`check-post-line-batch-pattern.bad.al`](check-post-line-batch-pattern.bad.al).
|
||||
|
|
|
|||
|
|
@ -26,10 +26,10 @@ For a master table, validate each shortcut dimension field through `ValidateDimV
|
|||
|
||||
For a document table, when the field that attaches the document to a master record changes (e.g. `Customer No.`), call `AddDimSource` naming that master table and key, then `GetDefaultDimID` to compute the document's new `Dimension Set ID`, inheriting the master's Default Dimension records. Pass `0` for `GetDefaultDimID`'s `InheritFromDimSetID` argument in this case — passing the document's *existing* `Dimension Set ID` instead inherits whatever dimensions were already in it, so a value the previous linked record supplied can survive into the new one even where the new record has no default for that dimension. Validate the document's own Shortcut Dimension fields through `ValidateShortcutDimValues`, which updates that same `Dimension Set ID` in place rather than persisting a separate Default Dimension record.
|
||||
|
||||
See sample: `dimension-management-wiring.good.al`.
|
||||
See sample: [`dimension-management-wiring.good.al`](dimension-management-wiring.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Adding a dimension-looking field with only a `TableRelation` to Dimension Value, and no call into `DimensionManagement` at all. The field accepts input but never becomes a real Default Dimension record, so it does not validate against blocked values and does not flow into postings.
|
||||
|
||||
See sample: `dimension-management-wiring.bad.al`.
|
||||
See sample: [`dimension-management-wiring.bad.al`](dimension-management-wiring.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ Once a table has shipped — to AppSource, or to any customer environment that h
|
|||
|
||||
Leave a published table's key exactly as shipped. Model a new discriminating dimension as a separate table with its own key instead of adding a field to the existing key, and branch orchestration code by the new dimension rather than filtering one shared table on an extra key field.
|
||||
|
||||
See sample: `do-not-change-primary-key.good.al`.
|
||||
See sample: [`do-not-change-primary-key.good.al`](do-not-change-primary-key.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Adding a field to a published table's primary or clustered key to distinguish a new case. This fails AppSource validation or any customer upgrade with `AS0009` as soon as rows already exist under the old key shape, whether the field is being added, removed, or reordered.
|
||||
|
||||
See sample: `do-not-change-primary-key.bad.al`.
|
||||
See sample: [`do-not-change-primary-key.bad.al`](do-not-change-primary-key.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ A procedure that reads a field from a setup or configuration table inside a bran
|
|||
|
||||
Once a business rule has decided that a setup-table field's value is required for a branch to behave correctly, call `TestField` on it before use, even though a plain read would "work" by returning a blank or zero without erroring. Write a test that blanks the setup field and asserts the resulting error, so the guard itself is verified rather than merely present.
|
||||
|
||||
See sample: `testfield-required-setup-field.good.al`.
|
||||
See sample: [`testfield-required-setup-field.good.al`](testfield-required-setup-field.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Reading a required setup-table field behind a presence check that falls through to a default value instead of erroring. This looks defensive because it never crashes, but it converts "administrator forgot to configure this" into "system silently did something else" — worse than a hard failure, because nobody is told anything went wrong.
|
||||
|
||||
See sample: `testfield-required-setup-field.bad.al`.
|
||||
See sample: [`testfield-required-setup-field.bad.al`](testfield-required-setup-field.bad.al).
|
||||
|
|
|
|||
|
|
@ -25,10 +25,10 @@ For document reports, prefer a `Word` layout (`DefaultRenderingLayout = Word`) o
|
|||
- [Creating an RDL layout report](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-report-performance)
|
||||
- [Troubleshooting reports / Report performance](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-reports-troubleshooting)
|
||||
|
||||
See sample: `document-report-word-layout.good.al`.
|
||||
See sample: [`document-report-word-layout.good.al`](document-report-word-layout.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Defaulting a document report's layout to RDLC out of habit or because a template happened to use it. This inherits RDLC's sandboxed-app-domain performance cost with no benefit tied to the report's actual content or calculation needs.
|
||||
|
||||
See sample: `document-report-word-layout.bad.al`.
|
||||
See sample: [`document-report-word-layout.bad.al`](document-report-word-layout.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ All AL identifiers — variables, procedures, parameters, fields, object names,
|
|||
|
||||
Write every identifier in English, and translate developer intent rather than transliterating it — when a requirement is described in another language, the resulting variable, procedure, and field names should still read as English. Captions and tooltips may carry target-language text in the source file, with locale translations managed through XLIFF.
|
||||
|
||||
See sample: `al-identifiers-english.good.al`.
|
||||
See sample: [`al-identifiers-english.good.al`](al-identifiers-english.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Using native-language identifiers such as a Danish variable or procedure name in AL source code, relying on the fact that the code still compiles and runs correctly. This makes the code unreadable to non-native-language contributors and mixes localization concerns into source that should stay language-neutral.
|
||||
|
||||
See sample: `al-identifiers-english.bad.al`.
|
||||
See sample: [`al-identifiers-english.bad.al`](al-identifiers-english.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ When a field or variable represents a genuine true/false state — yes/no, on/of
|
|||
|
||||
Type a field or variable as `Boolean` when the domain concept is inherently a true/false state. Do not replace a meaningful two-option domain model with a Boolean solely because it currently has two values.
|
||||
|
||||
See sample: `binary-choice-must-be-boolean.good.al`.
|
||||
See sample: [`binary-choice-must-be-boolean.good.al`](binary-choice-must-be-boolean.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Modeling a yes/no choice as an `Option` with two members, or as an `Integer` with magic-number values, forces every caller to remember which value means what and leaves room for a meaningless third value.
|
||||
|
||||
See sample: `binary-choice-must-be-boolean.bad.al`.
|
||||
See sample: [`binary-choice-must-be-boolean.bad.al`](binary-choice-must-be-boolean.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ When a variable or field represents a fixed set of named, mutually exclusive sta
|
|||
|
||||
Declare an `Enum` with named values and branch on the enum value, not a raw number.
|
||||
|
||||
See sample: `fixed-choice-set-must-use-enum-not-integer.good.al`.
|
||||
See sample: [`fixed-choice-set-must-use-enum-not-integer.good.al`](fixed-choice-set-must-use-enum-not-integer.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Using a plain `Integer` field with the meaning of each value tracked only in a comment pushes the documentation of the states into something the compiler cannot check and a future maintainer cannot rely on.
|
||||
|
||||
See sample: `fixed-choice-set-must-use-enum-not-integer.bad.al`.
|
||||
See sample: [`fixed-choice-set-must-use-enum-not-integer.bad.al`](fixed-choice-set-must-use-enum-not-integer.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ AL is case-insensitive, so `MESSAGE(...)`, `ERROR(...)`, `CONFIRM(...)`, and `ST
|
|||
|
||||
Call intrinsic functions in their modern, PascalCase form.
|
||||
|
||||
See sample: `intrinsic-al-functions-must-use-modern-casing.good.al`.
|
||||
See sample: [`intrinsic-al-functions-must-use-modern-casing.good.al`](intrinsic-al-functions-must-use-modern-casing.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
ALL-CAPS intrinsic function calls trip no compiler error, but they are a reliable signal that a code block was copied from old C/AL material or outdated training content rather than written against current AL conventions.
|
||||
|
||||
See sample: `intrinsic-al-functions-must-use-modern-casing.bad.al`.
|
||||
See sample: [`intrinsic-al-functions-must-use-modern-casing.bad.al`](intrinsic-al-functions-must-use-modern-casing.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ Since Business Central 2024 release wave 1, Microsoft's own objects are organize
|
|||
|
||||
When referencing an existing AL object, resolve its namespace from that object's actual source file or symbol definition — never infer or invent one from its name, functional area, or naming convention.
|
||||
|
||||
See sample: `namespace-must-be-verified-from-source.good.al`.
|
||||
See sample: [`namespace-must-be-verified-from-source.good.al`](namespace-must-be-verified-from-source.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Writing a `using` statement from memory, from an incomplete path, or from a plausible-looking guess. It can appear correct while actually resolving to the wrong object, or fail to resolve, once checked against stale or mismatched symbols, a different build configuration, or the object's actual current source — not because the compiler and the AL Language Server apply different namespace-resolution rules; they don't.
|
||||
|
||||
See sample: `namespace-must-be-verified-from-source.bad.al`.
|
||||
See sample: [`namespace-must-be-verified-from-source.bad.al`](namespace-must-be-verified-from-source.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ When a procedure declares a parameter with `var`, that parameter is passed by re
|
|||
|
||||
Declare a variable in the caller's scope and pass it to the `var` parameter.
|
||||
|
||||
See sample: `var-parameters-require-an-addressable-variable.good.al`.
|
||||
See sample: [`var-parameters-require-an-addressable-variable.good.al`](var-parameters-require-an-addressable-variable.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Passing a literal or a computed expression to a `var` parameter position fails to compile, because neither has an address the callee can write back to.
|
||||
|
||||
See sample: `var-parameters-require-an-addressable-variable.bad.al`.
|
||||
See sample: [`var-parameters-require-an-addressable-variable.bad.al`](var-parameters-require-an-addressable-variable.bad.al).
|
||||
|
|
|
|||
|
|
@ -21,10 +21,10 @@ Not every value should be generated, though. Incidental fixture data — identif
|
|||
|
||||
Use the standard library codeunits (`Library - ERM`, `Library - Inventory`, `Library - Sales`, `Library - Utility`) to generate incidental fixture values — they produce valid, unique-enough data via number series and controlled randomness, not a mathematical collision-free guarantee — and fill every mandatory field with correctly-sized data. Keep values that define the scenario's expected outcome explicit and fixed. Reserve hardcoded values for tests that validate an external contract itself — a fixed JSON schema, an EDIFACT message, a counterparty code — where the hardcoded value documents the specification rather than arbitrary test logic.
|
||||
|
||||
See sample: `test-data-must-be-random-and-complete.good.al`.
|
||||
See sample: [`test-data-must-be-random-and-complete.good.al`](test-data-must-be-random-and-complete.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Looking up a record assumed to already exist (a hardcoded payment method or customer number) instead of creating it, or leaving a mandatory field empty because setup-time validation happens to allow it. Also an anti-pattern, narrower: using a value that doesn't satisfy a scenario's explicit length or format requirement — for example a truncation test that never actually exceeds the field it's meant to overflow.
|
||||
|
||||
See sample: `test-data-must-be-random-and-complete.bad.al`.
|
||||
See sample: [`test-data-must-be-random-and-complete.bad.al`](test-data-must-be-random-and-complete.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ A field listed in `ODataKeyFields` cannot have `Editable = false` when the API p
|
|||
|
||||
Leave every consumer-supplied key field referenced in `ODataKeyFields` without `Editable = false` on pages where `InsertAllowed = true`, so the OData layer accepts it as a writable property on POST.
|
||||
|
||||
See sample: `api-page-key-fields-must-be-editable-on-insert.good.al`.
|
||||
See sample: [`api-page-key-fields-must-be-editable-on-insert.good.al`](api-page-key-fields-must-be-editable-on-insert.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Marking a consumer-provided key field `Editable = false`, out of habit or for perceived safety. This silently breaks create operations with a generic `BadRequest` instead of a clear validation error.
|
||||
|
||||
See sample: `api-page-key-fields-must-be-editable-on-insert.bad.al`.
|
||||
See sample: [`api-page-key-fields-must-be-editable-on-insert.bad.al`](api-page-key-fields-must-be-editable-on-insert.bad.al).
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ A stored field whose value is derived from other fields inside an `OnValidate` t
|
|||
|
||||
Recalculate the derived value in `OnAfterGetRecord` from its authoritative source — typically a FlowField — using a page-level variable, and expose that recalculated value instead of the stale stored field. Whether to also expose the source fields is a separate design decision, not a requirement of this pattern; keep the API contract scoped to what consumers actually need. If letting the consumer verify the recalculation is itself a requirement, expose every field the calculation reads, not just one of them — a derived value with two inputs needs both exposed, or the "verification" is incomplete.
|
||||
|
||||
See sample: `stored-derived-fields-must-not-be-exposed-directly.good.al`.
|
||||
See sample: [`stored-derived-fields-must-not-be-exposed-directly.good.al`](stored-derived-fields-must-not-be-exposed-directly.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Exposing the stored field directly via `Rec`, trusting that it was kept in sync by whichever trigger last touched it.
|
||||
|
||||
See sample: `stored-derived-fields-must-not-be-exposed-directly.bad.al`.
|
||||
See sample: [`stored-derived-fields-must-not-be-exposed-directly.bad.al`](stored-derived-fields-must-not-be-exposed-directly.bad.al).
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue