-
Notifications
You must be signed in to change notification settings - Fork 102
knowledge(performance): add community rules for performance #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Stefano Demiliani (demiliani)
wants to merge
3
commits into
microsoft:main
Choose a base branch
from
demiliani:community/performance-knowledge
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
24 changes: 24 additions & 0 deletions
24
community/knowledge/performance/avoid-currpage-update-in-onaftergetrecord.bad.al
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| page 50100 "CurrPage Update OAGR Bad" | ||
| { | ||
| PageType = List; | ||
| SourceTable = Customer; | ||
| ApplicationArea = All; | ||
|
|
||
| layout | ||
| { | ||
| area(content) | ||
| { | ||
| repeater(Rows) | ||
| { | ||
| field("No."; Rec."No.") { } | ||
| field(Name; Rec.Name) { } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| trigger OnAfterGetRecord() | ||
| begin | ||
| // Update from OnAfterGetRecord re-enters the trigger on every row. | ||
| CurrPage.Update(false); | ||
| end; | ||
| } |
26 changes: 26 additions & 0 deletions
26
community/knowledge/performance/avoid-currpage-update-in-onaftergetrecord.good.al
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| page 50100 "CurrPage Update OAGR Good" | ||
| { | ||
| PageType = List; | ||
| SourceTable = Customer; | ||
| ApplicationArea = All; | ||
|
|
||
| layout | ||
| { | ||
| area(content) | ||
| { | ||
| repeater(Rows) | ||
| { | ||
| field("No."; Rec."No.") { } | ||
| field(Warning; WarningText) { } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| var | ||
| WarningText: Text[50]; | ||
|
|
||
| trigger OnAfterGetRecord() | ||
| begin | ||
| WarningText := CopyStr(Rec.Name, 1, MaxStrLen(WarningText)); | ||
| end; | ||
| } |
28 changes: 28 additions & 0 deletions
28
community/knowledge/performance/avoid-currpage-update-in-onaftergetrecord.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| --- | ||
| bc-version: [all] | ||
| domain: performance | ||
| keywords: [currpage-update, onaftergetrecord, list-page, scroll, refresh] | ||
| technologies: [al] | ||
| countries: [w1] | ||
| application-area: [all] | ||
| --- | ||
|
|
||
| # Do not call CurrPage.Update inside OnAfterGetRecord | ||
|
|
||
| > Contributions welcome — open a PR to refine or extend this article. | ||
|
|
||
| ## Description | ||
|
|
||
| `OnAfterGetRecord` on a list already runs once per visible row on scroll and refresh. `CurrPage.Update` asks the page to reload, which fires those triggers again. The result is a refresh loop or a stutter on every row paint. Official developer performance guidance lists `CurrPage.Update()` in `OnAfterGetRecord` next to `Modify` as work that must not live there. Sibling of `do-not-modify-in-onaftergetrecord.md` (writes); this file is the client refresh half. | ||
|
|
||
| ## Best Practice | ||
|
|
||
| Put display-only results in page variables assigned in `OnAfterGetRecord` without calling `Update`. If the page must refresh after an action, call `CurrPage.Update(false)` from `OnAction` once, not per row. | ||
|
|
||
| See sample: `avoid-currpage-update-in-onaftergetrecord.good.al`. | ||
|
|
||
| ## Anti Pattern | ||
|
|
||
| `trigger OnAfterGetRecord() begin ... CurrPage.Update(); end;` on a list. The signal is `CurrPage.Update` inside `OnAfterGetRecord` or `OnAfterGetCurrRecord` without an explicit user action. | ||
|
|
||
| See sample: `avoid-currpage-update-in-onaftergetrecord.bad.al`. |
20 changes: 20 additions & 0 deletions
20
community/knowledge/performance/batch-number-series-instead-of-getnextno-per-row.bad.al
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| codeunit 50100 "Batch NoSeries Insert Bad" | ||
| { | ||
| procedure InsertDraftOrders(var Customer: Record Customer) | ||
| var | ||
| SalesHeader: Record "Sales Header"; | ||
| SalesSetup: Record "Sales & Receivables Setup"; | ||
| NoSeries: Codeunit "No. Series"; | ||
| begin | ||
| SalesSetup.Get(); | ||
| if Customer.FindSet() then | ||
| repeat | ||
| SalesHeader.Init(); | ||
| SalesHeader."Document Type" := SalesHeader."Document Type"::Order; | ||
| // Per-row GetNextNo locks the number-series line every insert. | ||
| SalesHeader."No." := NoSeries.GetNextNo(SalesSetup."Order Nos.", WorkDate()); | ||
| SalesHeader."Sell-to Customer No." := Customer."No."; | ||
| SalesHeader.Insert(true); | ||
| until Customer.Next() = 0; | ||
| end; | ||
| } |
20 changes: 20 additions & 0 deletions
20
community/knowledge/performance/batch-number-series-instead-of-getnextno-per-row.good.al
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| codeunit 50100 "Batch NoSeries Insert Good" | ||
| { | ||
| procedure InsertDraftOrders(var Customer: Record Customer) | ||
| var | ||
| SalesHeader: Record "Sales Header"; | ||
| SalesSetup: Record "Sales & Receivables Setup"; | ||
| NoSeriesBatch: Codeunit "No. Series - Batch"; | ||
| begin | ||
| SalesSetup.Get(); | ||
| if Customer.FindSet() then | ||
| repeat | ||
| SalesHeader.Init(); | ||
| SalesHeader."Document Type" := SalesHeader."Document Type"::Order; | ||
| SalesHeader."No." := NoSeriesBatch.GetNextNo(SalesSetup."Order Nos.", WorkDate()); | ||
| SalesHeader."Sell-to Customer No." := Customer."No."; | ||
| SalesHeader.Insert(true); | ||
| until Customer.Next() = 0; | ||
| NoSeriesBatch.SaveState(); | ||
| end; | ||
| } |
28 changes: 28 additions & 0 deletions
28
...unity/knowledge/performance/batch-number-series-instead-of-getnextno-per-row.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| --- | ||
| bc-version: [22..] | ||
| domain: performance | ||
| keywords: [no-series, getnextno, no-series-batch, savestate, numbersequence, lock] | ||
| technologies: [al] | ||
| countries: [w1] | ||
| application-area: [all] | ||
| --- | ||
|
|
||
| # Batch number-series calls instead of GetNextNo per insert | ||
|
|
||
| > Contributions welcome — open a PR to refine or extend this article. | ||
|
|
||
| ## Description | ||
|
|
||
| `Codeunit "No. Series".GetNextNo` on a **gapless (Normal)** series updates and locks the number-series line on every call. A tight `Insert` loop that asks for a number per row serializes every concurrent writer on that series — the classic SaaS posting bottleneck. Training data still copies the per-row C/AL `NoSeriesManagement` shape. Series configured with **Allow Gaps** instead obtain numbers through `NumberSequence` and do not hold the series-line lock between calls, so they are not affected by this pattern. Codeunit `"No. Series - Batch"` issues gapless numbers in memory and writes the series line once via `SaveState`. | ||
|
|
||
| ## Best Practice | ||
|
|
||
| Inside a multi-row insert, call `"No. Series - Batch".GetNextNo` per row and `SaveState` once after the loop when the series must remain gapless. Use `NumberSequence.Next` when holes are allowed. Do not replace a single `OnInsert` `GetNextNo` for one master record; that path is not the hotspot. | ||
|
|
||
| See sample: `batch-number-series-instead-of-getnextno-per-row.good.al`. | ||
|
|
||
| ## Anti Pattern | ||
|
|
||
| `NoSeries.GetNextNo(...)` inside `repeat ... Insert ... until Next() = 0` where the series is **gapless** (Allow Gaps = false). Each iteration takes the series-line lock. The signal is `"No. Series"` (not `"No. Series - Batch"`) in a loop that inserts more than one row; do not flag the same pattern when the series has Allow Gaps enabled, as the `NumberSequence` path already avoids the lock. | ||
|
|
||
| See sample: `batch-number-series-instead-of-getnextno-per-row.bad.al`. |
19 changes: 19 additions & 0 deletions
19
community/knowledge/performance/changecompany-in-loop-drops-caches.bad.al
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| codeunit 50100 "ChangeCompany Loop Bad" | ||
| { | ||
| procedure NamesForCustomers(var Buffer: Record Customer) | ||
| var | ||
| Customer: Record Customer; | ||
| Company: Record Company; | ||
| begin | ||
| if Buffer.FindSet() then | ||
| repeat | ||
| if Company.FindSet() then | ||
| repeat | ||
| // ChangeCompany per customer per company resets caches every row. | ||
| Customer.ChangeCompany(Company.Name); | ||
| if Customer.Get(Buffer."No.") then | ||
| Message(Customer.Name); | ||
| until Company.Next() = 0; | ||
| until Buffer.Next() = 0; | ||
| end; | ||
| } |
25 changes: 25 additions & 0 deletions
25
community/knowledge/performance/changecompany-in-loop-drops-caches.good.al
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| codeunit 50100 "ChangeCompany Loop Good" | ||
| { | ||
| procedure NameInCompany(CompanyNameValue: Text[30]; CustomerNo: Code[20]): Text | ||
| var | ||
| Customer: Record Customer; | ||
| begin | ||
| Customer.ChangeCompany(CompanyNameValue); | ||
| Customer.SetLoadFields(Name); | ||
| if Customer.Get(CustomerNo) then | ||
| exit(Customer.Name); | ||
| end; | ||
|
|
||
| procedure NamesForCompanies(var Company: Record Company) | ||
| var | ||
| Customer: Record Customer; | ||
| begin | ||
| if Company.FindSet() then | ||
| repeat | ||
| Customer.ChangeCompany(Company.Name); | ||
| Customer.SetLoadFields(Name); | ||
| if Customer.FindFirst() then | ||
| Message(Customer.Name); | ||
| until Company.Next() = 0; | ||
| end; | ||
| } | ||
28 changes: 28 additions & 0 deletions
28
community/knowledge/performance/changecompany-in-loop-drops-caches.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| --- | ||
| bc-version: [all] | ||
| domain: performance | ||
| keywords: [changecompany, loop, cache, multi-company, isolation] | ||
| technologies: [al] | ||
| countries: [w1] | ||
| application-area: [all] | ||
| --- | ||
|
|
||
| # Do not call ChangeCompany inside a per-row loop | ||
|
|
||
| > Contributions welcome — open a PR to refine or extend this article. | ||
|
|
||
| ## Description | ||
|
|
||
| `ChangeCompany` retargets a record variable to another company's data and drops the in-memory caches bound to the previous company. Calling it once per row in a multi-company scan therefore pays a cache reset on every iteration, even when consecutive rows share a company. Agents treat `ChangeCompany` like a filter. It is an isolation switch. | ||
|
|
||
| ## Best Practice | ||
|
|
||
| Group work by company. Call `ChangeCompany` once per distinct company, then `FindSet`/`Get` that company's rows. Reset the variable back when the batch finishes. | ||
|
|
||
| See sample: `changecompany-in-loop-drops-caches.good.al`. | ||
|
|
||
| ## Anti Pattern | ||
|
|
||
| `repeat Rec.ChangeCompany(Buffer.Company); Rec.Get(Buffer."No."); until Buffer.Next() = 0` when `Buffer` is not ordered by company, or even when it is — if `ChangeCompany` still runs every row. The signal is `ChangeCompany` inside `repeat`/`while` keyed by a document line rather than by a company loop. | ||
|
|
||
| See sample: `changecompany-in-loop-drops-caches.bad.al`. |
22 changes: 22 additions & 0 deletions
22
community/knowledge/performance/countapprox-for-progress-not-count.bad.al
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| codeunit 50100 "CountApprox Progress Bad" | ||
| { | ||
| procedure RecalcUsCustomers() | ||
| var | ||
| Customer: Record Customer; | ||
| Window: Dialog; | ||
| Counter: Integer; | ||
| Total: Integer; | ||
| begin | ||
| Customer.SetRange("Country/Region Code", 'US'); | ||
| // Exact Count() is a SELECT COUNT(*) just to drive a progress bar. | ||
| Total := Customer.Count(); | ||
| Window.Open('Processing #1###### of #2######'); | ||
| if Customer.FindSet() then | ||
| repeat | ||
| Counter += 1; | ||
| Window.Update(1, Counter); | ||
| Window.Update(2, Total); | ||
| until Customer.Next() = 0; | ||
| Window.Close(); | ||
| end; | ||
| } |
21 changes: 21 additions & 0 deletions
21
community/knowledge/performance/countapprox-for-progress-not-count.good.al
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| codeunit 50100 "CountApprox Progress Good" | ||
| { | ||
| procedure RecalcUsCustomers() | ||
| var | ||
| Customer: Record Customer; | ||
| Window: Dialog; | ||
| Counter: Integer; | ||
| Total: Integer; | ||
| begin | ||
| Customer.SetRange("Country/Region Code", 'US'); | ||
| Total := Customer.CountApprox(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as count() |
||
| Window.Open('Processing #1###### of #2######'); | ||
| if Customer.FindSet() then | ||
| repeat | ||
| Counter += 1; | ||
| Window.Update(1, Counter); | ||
| Window.Update(2, Total); | ||
| until Customer.Next() = 0; | ||
| Window.Close(); | ||
| end; | ||
| } | ||
28 changes: 28 additions & 0 deletions
28
community/knowledge/performance/countapprox-for-progress-not-count.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| --- | ||
| bc-version: [all] | ||
| domain: performance | ||
| keywords: [countapprox, count, dialog, progress-bar, approximate-count] | ||
| technologies: [al] | ||
| countries: [w1] | ||
| application-area: [all] | ||
| --- | ||
|
|
||
| # Use CountApprox for progress UI, not Count | ||
|
|
||
| > Contributions welcome — open a PR to refine or extend this article. | ||
|
|
||
| ## Description | ||
|
|
||
| `Count()` asks SQL for an exact row count of the current filter. When no SIFT key covers all filtered fields, this is a `SELECT COUNT(*)` against the data rows before any useful work starts — the usual cost of `Dialog.Open` with a percentage bar. (A filtered count on a table with a matching SIFT key is cheap, but SIFT coverage cannot be assumed for arbitrary filters.) `CountApprox()` exists for the progress-UI case: it returns a cheap estimate (partition stats / metadata), accurate enough for a progress denominator. Agents default to `Count()` because the name matches "how many rows". | ||
|
|
||
| ## Best Practice | ||
|
|
||
| Feed progress dialogs and informational messages with `CountApprox()`. Use `Count()` only when the exact integer is a business result (a posted control, a reconciliation, a test assertion). | ||
|
|
||
| See sample: `countapprox-for-progress-not-count.good.al`. | ||
|
|
||
| ## Anti Pattern | ||
|
|
||
| `Total := Rec.Count(); Window.Open(...);` immediately before a `FindSet` over the same filter. The exact count is discarded after the bar finishes; when the filter is not covered by a SIFT key, the user paid a full table scan just to draw the progress bar. | ||
|
|
||
| See sample: `countapprox-for-progress-not-count.bad.al`. |
15 changes: 15 additions & 0 deletions
15
community/knowledge/performance/dataaccessintent-readonly-on-analytical-objects.bad.al
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| report 50100 "Cust List ReadOnly Bad" | ||
| { | ||
| UsageCategory = ReportsAndAnalysis; | ||
| ApplicationArea = All; | ||
| // Missing DataAccessIntent = ReadOnly; the scan hits the primary replica. | ||
|
|
||
| dataset | ||
| { | ||
| dataitem(Customer; Customer) | ||
| { | ||
| column(No; "No.") { } | ||
| column(Name; Name) { } | ||
| } | ||
| } | ||
| } |
15 changes: 15 additions & 0 deletions
15
community/knowledge/performance/dataaccessintent-readonly-on-analytical-objects.good.al
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| report 50100 "Cust List ReadOnly Good" | ||
| { | ||
| UsageCategory = ReportsAndAnalysis; | ||
| ApplicationArea = All; | ||
| DataAccessIntent = ReadOnly; | ||
|
|
||
| dataset | ||
| { | ||
| dataitem(Customer; Customer) | ||
| { | ||
| column(No; "No.") { } | ||
| column(Name; Name) { } | ||
| } | ||
| } | ||
| } |
28 changes: 28 additions & 0 deletions
28
community/knowledge/performance/dataaccessintent-readonly-on-analytical-objects.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| --- | ||
| bc-version: ["16.."] | ||
| domain: performance | ||
| keywords: [dataaccessintent, read-only, read-scale-out, report, api-page, query] | ||
| technologies: [al] | ||
| countries: [w1] | ||
| application-area: [all] | ||
| --- | ||
|
|
||
| # Set DataAccessIntent ReadOnly on analytical objects | ||
|
|
||
| > Contributions welcome — open a PR to refine or extend this article. | ||
|
|
||
| ## Description | ||
|
|
||
| `DataAccessIntent` was introduced at runtime 5.0 (BC 16) and has no effect in earlier versions. Reports, API pages (`PageType = API` with `Editable = false`), and queries that only read can run against a read replica when `DataAccessIntent = ReadOnly`. For queries, replica routing only applies when the query is exposed via OData/API; running a query in AL code is unaffected. Without the property these objects hit the primary replica and compete with posting. Agents omit it because the default is read-write and the object "only reads" in AL. The replica routing is a metadata switch, not something the compiler infers from the absence of `Modify`. | ||
|
|
||
| ## Best Practice | ||
|
|
||
| On report objects and `PageType = API` pages with `Editable = false` that never write, set `DataAccessIntent = ReadOnly`. For query objects, set it when the query is consumed via OData or an API endpoint. Keep the default on objects that insert, modify, or call a write codeunit from a processing-only report. | ||
|
|
||
| See sample: `dataaccessintent-readonly-on-analytical-objects.good.al`. | ||
|
|
||
| ## Anti Pattern | ||
|
|
||
| A listing report or API query with no `DataAccessIntent` that scans G/L or sales lines. The object is read-only in practice and still loads the primary. | ||
|
|
||
| See sample: `dataaccessintent-readonly-on-analytical-objects.bad.al`. |
24 changes: 24 additions & 0 deletions
24
community/knowledge/performance/guiallowed-guard-on-pages-used-as-odata.bad.al
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| page 50100 "GuiAllowed OData Guard Bad" | ||
| { | ||
| PageType = List; | ||
| SourceTable = Customer; | ||
| ApplicationArea = All; | ||
|
|
||
| layout | ||
| { | ||
| area(content) | ||
| { | ||
| repeater(Rows) | ||
| { | ||
| field("No."; Rec."No.") { } | ||
| field(Name; Rec.Name) { } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| trigger OnAfterGetRecord() | ||
| begin | ||
| // Runs for every OData / Edit-in-Excel row with no UI. | ||
| Rec.CalcFields("Balance (LCY)"); | ||
| end; | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this pattern is better regarding change company - other than the SetLoadFields, which perhaps could have been set outside the loop. Is the function NameInCompany used?