If not find then exit

[Anders-Ryden]

https://alguidelines.dev/docs/bestpractices/if-not-find-then-exit/

This rule suggest that we should check if the record is empty, and if so exit the code instead of checking for records and then looping.
This should give clearer code and reduce the number of ends and extra indentations.

if Record.Findset() then begin
    repeat
        DoStuff(Record);
    until Record.Next() = 0;
end;

should be replaced with

if not Record.Findset() then 
    exit();

repeat
    DoStuff(Record);
until Record.Next() = 0;

Only I have never written a loop like that, there is no need for the begin/end so I write it like this.

if Record.Findset() then repeat
    DoStuff(Record);
until Record.Next() = 0;

To my mind that is clearer.
I don't see the benefit here, I prefer to make positive code that runs all the way rather than negative code where you exit the code at various places along the way if that makes sense.

What are your thoughts?

[TheDoubleH]

I actually think this should really have been called Early Exit Early Exit

The Point with this pattern is really to exit a function early, in order to avoid indented code.

if Record.Findset() then begin
    repeat
        DoStuff(Record);
    until Record.Next() = 0;
end;

should be replaced with

if not Record.Findset() then exit();

repeat
    DoStuff(Record);
until Record.Next() = 0;

I do not see that reference, but it really should be:

if not Record.Findset() then
    exit;

repeat
    DoStuff(Record);
until Record.Next() = 0;

[Anders-Ryden]

Yes, I corrected that line break in the original post.

But what is the benefits of early exit in this case?
I get it in case of big nested if clauses and other code like it, do your tests and get out as fast as possible if they fail, saves execution time and make it easy to follow requirements.

But what would be the advantage of this way of writing it

if not Record.Findset() then
    exit;

repeat
    DoStuff(Record);
until Record.Next() = 0;

over this way?

if Record.Findset() then repeat
    DoStuff(Record);
until Record.Next() = 0;

I feel that it would just increase the possibility that further down the line we'll change something and might end up with dead code.
I see this as beeing about readability, for me an if Record.findset then repeat is clearer than a repeat on its own, that, to me, implies code that should always be run.

[NP-AlexTrimberger]

But what is the benefits of early exit in this case? I get it in case of big nested if clauses and other code like it, do your tests and get out as fast as possible if they fail, saves execution time and make it easy to follow requirements.

Consistency - why have two approaches for the same thing?

The biggest benefit for me is that using early exit forces you to refactor large blocks of code into smaller procedures.

It's easy to shove this code into the middle of a large block without breaking anything.

if Record.Findset() then repeat
    DoStuff(Record);
until Record.Next() = 0;

It's easy to write an unmaintainable mess with the 'positive path'

In comparison, it's not easy to shove this code into the middle of a large block, or write a massive procedure that does lots of different things -

if not Record.Findset() then
    exit;

repeat
    DoStuff(Record);
until Record.Next() = 0;

It will need it's own procedure, which means that our solution will be more modular, with smaller procedures that have fewer responsibilities.

[KOTRET]

While you try to sell the early-exit-pattern as a benefit that forces you to "modularize" your app, you provide an example that shows that this pattern is prone to errors when it comes to refactoring.

[KOTRET]

if ... then opens a code block and repeat ... until does that too, so imho it's right to place them on separate lines (as this also indents correctly).

*nb: moved comment into separate comment to avoid mixed conversation

[Anders-Ryden]

Ok, I see a valid point in early exit strategy, but my original question was not really about that but what @KOTRET adress above.

if Record.Findset() then begin
        repeat
                DoStuff(Record);
        until Record.Next() = 0;
end;

VS

if Record.Findset() then 
        repeat
                DoStuff(Record);
        until Record.Next() = 0;

I see no reason for the begin..end block
Maybe I'm just a dinosaur who still uses C/AL syntax, but MS still uses that pattern themselves today, CU80 is full of if findset then repeat...

[NKarolak]

I see no reason for the begin..end block

I agree. And it also violates CodeCop rule AA0005: Only use BEGIN..END to enclose compound statements.

[KOTRET]

i thought it was about

if ... then repeat
    ....
until

instead of

if ... then
    repeat
    ...
    until