34
from closed AL issue https://github.com/microsoft/AL/issues/5727

***

Summary:

There are standard AL warnings if FindFirst() is used without then checking any fields from the found record. But it is in my experience very common to see both Microsoft and third-party devs like myself using FindFirst() as a quick and extremely convenient way to say 'if no record exists within these filters, error out with explanation'. The same goes for Get().

Having to replace these with optimised code that doesn't load fields, by using IsEmpty(), is tedious: to replace FindFirst(), we need to replicate the standard error and its substitution of the current filters, and to replace Get() is is even worse because we have to translate it to SetRange()s + FindFirst().

Erroring if a record does not exist, either within a filter or by PK fields, is such a common check that it should be supported by the platform, without having to roll our own, load fields that won't be used, get a warning, etc. Thus, I propose the addition of new methods for this to the Record data type (with naming flexible of course):

* Record.TestNotEmpty() - which will effectively check whether IsEmpty() and if so give the same error as FindFirst() would, but without loading any fields or raising a warning.
* Record.TestExists(Primary, Key, Fields) - which will check whether Record.Get(Primary, Key Fields) would succeed and if not give the same error as Get() would, but without loading any fields or raising a warning.

I believe they will greatly improve developer/user/infrastructure experience, as devs will then be able to use the same kind of terse syntax to give users the standard platform errors, but with less overhead on SQL from fields.

So it's really a no-brainer ;-) Please strongly consider adding these. Thanks!

***

further discussion from the AL issue:

PaulIvan said:
>>>
we strongly believe that if a record's content is not needed, findfirst/findset should not be used.

In this case if you just want to check if a record exists, then IsEmpty() should be used.
>>>

I said:
>>>
It is a highly common pattern to use `Record.FindFirst()` to mean 'if no record exists within the filter, give a standard error'.

Having to use `Record.IsEmpty()` then means having to roll our own error message label every time, or call out to some shared function every time - neither ideal. For as much as users can't grok standard platform errors, they do make developers' lives *much* easier!

Of course, it's wasteful to load all the fields (unless we use e.g. `Record.SetLoadFields(SystemId)` to defeat that), but often that doesn't seem harmful enough to performance to outweigh how convenient `FindFirst()` is to get such a check.

Thus, if you are going to warn about such usage, can you please give an alternative, such as `Record.TestNotEmpty()`, which will throw the standard error but not load any fields?

An equivalent might also be needed for `Record.Get(Key, Fields)` if that is also just to check existence - such as `Record.TestExists(Key, Fields)`.

Please consider these, since as mentioned, many devs use them to get standard error checking and reporting, and having to roll our own errors every time would be nightmarish.
>>>

PaulIvan said:
>>>
You know this as well as I, just because something is popular it doesn't mean that it is the right thing, the spirit of this rule is to limit reads when they are not needed and while I agree that it is way easier to just use findfirst and etc to trigger platform errors when records are not found it is not in the spirit of the rule and that is the best we have for now and of course people can bypass it if it doesn't suit their need.

That said, I think you do have a great point for the platform to extend capabilities with more functions that trigger standard errors. Feel free to make a suggestion to the platform team.
>>>
Category: Development
STATUS DETAILS
Needs Votes
Ideas Administrator

Thank you for this suggestion! Currently this is not on our roadmap. We are tracking this idea and if it gathers more votes and comments we will consider it in the future. Best regards, Business Central Team

Comments

D

This would indeed be good to have. Currently in these cases we apply a SetLoadFields on a primary key field to minimize the things fields that are loaded and prevent joins of extension tables. However code would improve if we just could use a specific function instead.

Category: Development