Recommend using <Set|Add>LoadFields

[someC0d3r]

Currently the CodeCop warns you if you use a field which is not loaded in the current scope, but this warning only occurs if you have SetLoadFields in your code scope - if you don't have it in the scope, the warning doesn't show up / it doesn't get checked

Hint: If you change SetLoadFields with AddLoadFields the warning also doesn't show up - so the trigger seems to be SetLoadFields for this CodeCop rule

The idea:

1. Create a new rule which recommends you using SetLoadFields if you use for example <Record|RecordRef>.Find, FindSet, FindFirst, FindLast, Next (maybe more) and a field of it
2. This rule should also work if you pass the record/recordref variable to a procedure (AA0242 doesn't check this unfortunately :( )

Example for 2.:

It doesn't give you the warning in the method :/

Reason:
To make the performance of your solution / app better by using <Set|Add>LoadFields this rule would be a big help.

Greets
someC0d3r :)

[StefanMaron]

Yeah, I thought a lot about a rule like this recently. But it gets more complicated the longer you think about it.

the rule should then also be able to trace variables if they are passed by reference, that only works until the call path leaves the current extension.
I am not even sure how that would be possible in the analyzer

[someC0d3r]

Yea... the struggle about tracing the variables came in my mind too.
Variable "SalesCommentLine" -> SomeFunction(SalesCommentLine) -> SomeFunction2(SalesCommentLine) - and so on..

If this rule will get implemented it will be a bit "hackish" I suppose :/

[fvet]

Also some other restrictions apply if the Findx would be followed by a Modify, ...:

For performance reasons, it's not recommended to use partial records on a record that will do inserts, deletes, renames, field transfers, or copies to temporary records. All these operations require that all fields on the record are loaded, so the platform will emit a JIT load if they're not already loaded.

[fvet]

Maybe we should start 'small' first and limit the scope of the warning to below usages?

• Focus on 'local' record variables only
• Record variable should be subject of a 'find' procedure (findfirst, findset, findlast ...) (no isempty) -> read
• Record variable should not have any function call (no Insert, Modify, Delete, Modifyall, deleteall, ... nor any procedure on the table itself) -> no involved in any updates
• Record variable should not be used as argument in any function calls -> no references
• Record variable should not be used as argument in any event publishers -> no references

This would at least show the occurrences where a record is fetched to only read data / get a field value.

https://www.yammer.com/dynamicsnavdev/#/Threads/show?threadId=1033577983172608

[lv-janpieter]

Maybe we should start 'small' first and limit the scope of the warning to below usages?

• Focus on 'local' record variables only
• Record variable should be subject of a 'find' procedure (findfirst, findset, findlast ...) (no isempty) -> read
• Record variable should not have any function call (no Insert, Modify, Delete, Modifyall, deleteall, ... nor any procedure on the table itself) -> no involved in any updates
• Record variable should not be used as argument in any function calls -> no references
• Record variable should not be used as argument in any event publishers -> no references

This would at least show the occurrences where a record is fetched to only read data / get a field value.

https://www.yammer.com/dynamicsnavdev/#/Threads/show?threadId=1033577983172608

That is exactly what we check in our code reviews.
SetLoadFields is mandatory, but only for records where scope does not extend beyond the function it is declared in.

• It cannot be passed into functions.
• You cannot call functions on the table, that includes modify/insert/delete/validate etc.

Any rule checking beyond the scope of the current procedure sound really complicated, if not impossible.

[DarleenN]

Has there been any Update to this Topic? In this post https://www.keytogoodcode.com/post/jit-loads-vs-full-record-load was shown that SetLoadFields is allways faster than working with the whole Table so the relevance for this rule is higher than ever.

Greetings :)

[pri-kise]

I would like to suggest an addtional check for the LoadFields.

Currently there are some places where Microsoft by default only loads partial records
e.g. in Reports https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-partial-records#reports

Currently there isn't a warning that reminds me to add AddLoadFields on the OnPreDataItem to for fields that aren't used in columns.

we have e.g. some processing only reports that generates excel files.

[lv-janpieter]

I would like to suggest an addtional check for the LoadFields.

Currently there are some places where Microsoft by default only loads partial records e.g. in Reports https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-partial-records#reports

Currently there isn't a warning that reminds me to add AddLoadFields on the OnPreDataItem to for fields that aren't used in columns.

we have e.g. some processing only reports that generates excel files.

I already made a separate discussion about this:
#1167
If you like the idea, vote it up.

[Arthurvdv]

Good news, this idea didn’t get lost and it’s been picked up and is now an implemented in ALCops. I've focused on the SetLoadFields, a. AddLoadFields will be another separate diagnostic.

You can already try it out today in the current beta while we’re working towards the stable v0.7.0 release. Details on the new diagnostic is available here: https://alcops.dev/docs/analyzers/platformcop/pc0030/.

Also, as noted on the front page of this repository:

Important

This repository is deprecated and will not receive any new updates for the foreseeable future.
Development continues at ALCops (GitHub).
If you would like to contribute, please do so in one of the active repositories there.

With that in mind, I’m go ahead and close this discussion here and we could continue the conversation over at: https://github.com/ALCops/Analyzers/discussions

Thanks!