mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Merge pull request #119 from microsoft/bcq/batch2-fp-guards
Add batch-2 FP guards: sentence-case action captions + PK Get is transaction-cached
This commit is contained in:
commit
ad8ccde595
3 changed files with 88 additions and 0 deletions
|
|
@ -0,0 +1,24 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: performance
|
||||||
|
keywords: [get, primary-key, record-cache, transaction, n-plus-one, dictionary-cache, over-engineering, false-positive]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# A primary-key Get() in a per-row helper is not an N+1 to cache manually
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
The Business Central server caches primary-key reads within a transaction. Repeated `Record.Get(<primary key>)` calls for the same key are served from that cache rather than re-queried, so a guarded `if not Rec.Get(...) then exit;` inside a per-row helper is not a genuine N+1 pattern. When each row legitimately carries a distinct key — for example one `Bin Content` row per bin, so `Bin.Get` and `BinType.Get` see a different bin each iteration — the `Get` must run per row regardless, and there is nothing to hoist.
|
||||||
|
|
||||||
|
Reviewers sometimes see two `Get` calls inside a routine that runs once per row and recommend wrapping them in a `Dictionary` cache. That is over-engineering: it duplicates the server's built-in record cache, adds state that must be invalidated, and breaks the surrounding extension's established pattern of direct guarded `Get` calls.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Treat a primary-key `Get()` — especially a guarded `if not Rec.Get(...) then exit;` — as a cheap, transaction-cached read. Do not recommend a manual `Dictionary` cache around per-row primary-key `Get` calls. Reserve N+1 concerns for genuinely repeated non-keyed queries (`FindSet`/`FindFirst` with filters, `Count`) that re-hit the database each iteration.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Reporting repeated primary-key `Get` calls (such as `Bin.Get` and `BinType.Get`) inside a per-row helper as a performance defect, or recommending they be cached in a `Dictionary`. The reads are already cached by the server within the transaction, and per-row keys often differ so the calls cannot be hoisted.
|
||||||
|
|
@ -0,0 +1,38 @@
|
||||||
|
page 50210 "UI Sample Caption Case"
|
||||||
|
{
|
||||||
|
PageType = List;
|
||||||
|
ApplicationArea = All;
|
||||||
|
SourceTable = "Sales Line";
|
||||||
|
|
||||||
|
layout
|
||||||
|
{
|
||||||
|
area(Content)
|
||||||
|
{
|
||||||
|
repeater(Lines)
|
||||||
|
{
|
||||||
|
field("Document No."; Rec."Document No.")
|
||||||
|
{
|
||||||
|
ToolTip = 'Specifies the document number.';
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
actions
|
||||||
|
{
|
||||||
|
area(Processing)
|
||||||
|
{
|
||||||
|
action(ShowSourceDocument)
|
||||||
|
{
|
||||||
|
Caption = 'Show source document';
|
||||||
|
Image = ViewSourceDocumentLine;
|
||||||
|
ToolTip = 'Open the related source document.';
|
||||||
|
|
||||||
|
trigger OnAction()
|
||||||
|
begin
|
||||||
|
Message('%1', Rec."Document No.");
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: ui
|
||||||
|
keywords: [caption, capitalization, sentence-case, title-case, action, noun-phrase, false-positive]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Sentence-phrase captions use sentence case, not title case
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Business Central caption capitalization depends on whether the caption reads as a **noun phrase** or a **sentence/verb phrase**. Following the Microsoft writing-style guideline, a caption that reads as an imperative sentence — most action captions, such as `'Show source document'`, `'Post and print'`, or `'Copy from last inspection'` — uses **sentence case**: only the first word and any proper nouns are capitalized. Title case (`'Show Source Document'`) is the older convention and is not required for these captions.
|
||||||
|
|
||||||
|
Noun-phrase captions (object names, field labels such as `'Source Document No.'`) follow their own capitalization; that is a separate case and is not what this article covers. Reviewers sometimes see a lower-cased word in an action caption (`'Show source document'`) and flag it as inconsistent title case, but a sentence-phrase action caption is correct as written.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
For an action `Caption` that reads as a sentence or verb phrase, capitalize only the first word and proper nouns (sentence case). Do not require every significant word to be capitalized. Before flagging a caption as "should be title case", confirm it is a noun phrase; leave imperative/sentence-phrase action captions in sentence case.
|
||||||
|
|
||||||
|
See sample: `caption-capitalization-noun-phrase-vs-sentence-phrase.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Reporting a sentence-case action caption such as `'Show source document'` as a style defect and recommending title case (`'Show Source Document'`), or calling it inconsistent with BC conventions. Sentence case is the current guideline for sentence-phrase captions.
|
||||||
Loading…
Add table
Add a link
Reference in a new issue