AND Operator

ta5

Hi Experts

As far as I see, NAV doesn't stop evaluating an AND statement after the first wrong expression.

Example:
IF expression1 AND expression2 THEN...

Imho, expression2 is evalutated even expression1 is FALSE.

The question:
If expression2 is an expensive function, for example with DB access, is it better to write like that?

IF expression1 THEN
IF expression2 THEN

This way expression2 is only evaluated if expression1 is TRUE.

I guess this is a bit of a philosophic question, but anyway I shall be glad to get some info about your experience and/or best practice.

Kind regards
Thomas

lynge

Correct.
I also discovered that when I started to code in NAV in 2013.

So NAV does NOT do short-circuit evaluation (https://en.wikipedia.org/wiki/Short-circuit_evaluation).

As a developer you need to be aware of this, so you don't make mistakes like:
IF(Customer.GET('xxx') AND CallSomeFunction(Customer))...

In NAV CallSomeFunction WILL be called even if Customer does not exist. In (most?) other programming languages it will not be called.

So the correct NAV code should probably read:
IF Customer.GET('xxx') THEN
IF CallSomeFunction(Customer)...

Off cause you can construct lots of similar examples.

It will probably never be changed as it potentially will break lots of existing (partner) code...

lynge

Actually you should also be aware that NAV will always evaluate both expressions in:
if (<expression 1> OR <expression 2>)

Even when <expression 1> is evaluated as true, NAV will still evaluate <expression 2>.
That is also not always the case if short-circuit evaluation is used...

ta5

Hi lynge
Thanks for confirmation of the behaviour. The OR stuff is also worth noticing.
What is your experience concerning "ANDing" of performance demanding expressions?
Thanks
Thomas

Slawek_Guzek

If you evaluate simple variables impact it neglible. If you get single records like in IF Customer.GET().. the impact is also not massive. But sometimes there are function calls directly used in IF expression, like

IF  Customer.GET('somecust') AND (GetCustomerBalance('somecust') < 0 ) THEN..

In such a case GetCustomerBalance() fill be always fired, and since this one can be a bit heavy it may have visible impact.

There are 2 general ways of coding this:

IF Customer.GET('somecust') THEN
  IF (GetCustomerBalance('somecust') < 0 ) THEN..

or (my favourite for replaing long AND... AND ... AND... conditions)

CASE TRUE OF
  Customer.GET('somecust') : ; //do nothing or call some specific missing customer handler
  GetCustomerBalance('somecust') < 0 : DoSomething();
  ESLE
    DoSomethingElse();
END;

My favourite is my favourite because it is shorter than IF.. THEN IF THEN .. ELSE ...ELSE..., which helps especially if there many parts in the condition expression, it clearly defines execution order, and it helps avoiding repetitions.

Slawek

vaprog

Yet another way to force shortcut evaluation for AND is the following:

IF TRUE IN [Expression1, Expression2, ...] THEN
  DoSomething
ELSE
  DoSomethingElse;

The CASE statement by Slawek above does not do the right thing. It should be

CASE FALSE OF
  Customer.GET('somecust'),
  GetCustomerBalance('somecust') < 0 :
    DoSomethingElse();
  ELSE
    DoSomething();
END;

Shortcut evaluation of OR does not work with IN, but you can do it with CASE like so:

CASE TRUE OF
  Expression1,
  Expression2,
  ...,
  ExpressionN :
    DoSomeThing;
  ELSE
    DoSomethingElse;
END;

As you can see, the AND replaced with CASE is a little awkward because Then and Else cases are reversed. So I tend to use IN, but only when I need shortcut evaluation.

I really prefer CASE over OR as soon as there are more then 2 or 3 terms involved, even if I don't need the shortcut evaluation.

Slawek_Guzek

True, there s a bug in my CASE example, it should be NOT Customer.GET(''') :

But the example below is also incorrect:

vaprog wrote: »

The CASE statement by Slawek above does not do the right thing. It should be

CASE FALSE OF
  Customer.GET('somecust'),
  GetCustomerBalance('somecust') < 0 :
    DoSomethingElse();
  ELSE
    DoSomething();
END;

Using a comma in CASE conditions in just like using IN [expr1, expr2] - both conditions are evaluated, which means, if you used functions in them, both will be executed. And this is exactly what we want to avoid by using CASE

To get exact equivalent of

IF Customer.GET(GetCustomerBalance('somecust') AND (GetCustomerBalance('somecust')  < 0) THEN

and to get 'short-circut evaluation' using CASE it needs to be this way:

CASE TRUE OF
  NOT Customer.GET('somecust') : ; //I have forgotten NOT in my earlier example
  GetCustomerBalance('somecust') < 0 : DoSomething();
  ELSE
    DoSomethingElse();
END;

or this

CASE FALSE OF
  Customer.GET('somecust') : ; //exit here if Customer.GET(..) fails
  GetCustomerBalance('somecust') >= 0 : //smth < 0 = TRUE is the same as smth >=0 = FALSE
    DoSomethingElse();
  ELSE
    DoSomething();
END;

Slawek

David_Singleton

Always remember the KISS rule in Navision. It is not a competition to make the code short and complex, its about performance and readability

IF A then
  IF B Then Begin
    MyCode;
  END;

The fact that two very senior developers were able to both write bugs into a very simple piece of code shows why we keep it simple.

kriki

[Topic moved from 'NAV Three Tier' forum to 'NAV Tips & Tricks' forum]


BTW: it is also the case with the classic client.

ta5

Thanks for your answers. I'm quite excited other devs have the same ideas in mind
Good for now.
Thanks again.
Heinz

vaprog

Slawek_Guzek wrote: »

Using a comma in CASE conditions in just like using IN [expr1, expr2] - both conditions are evaluated

That is actually not how this is evaluated. Instead, the first match is taken and all set members after that match are not evaluated.

You may easily test this out verifying the side effects of the functions, or tracing the code in the debugger.

My code using IN was wrong, though, as was my statement about using IN for OR shortcut evaluation. For both approaches, CASE, and IN, the logic is the same.
To get an AND shortcut evaluation you test for FALSE and reverse the THEN end ELSE parts in respect to the simple IF statement.
To get an OR shortcut evaluation you simply test for TRUE.
The AND case, therefore is really awkward and unintuitive, but if you need the ELSE part, it might still be preferable to David's simple cascade of IF statements, which definitely is more readable, as long as there are not too many terms in the AND expression, and as long there is no ELSE part you would not want to repeat several times for each nested IF.

The colon after the first value set {[NOT] Customer.GET('somecust') : } causes nothing to be executed if it triggers (actually it causes a syntax error, a possibly empty statement followed by a semicolon is required after it). What should happen, though, is DoSomethingElse() to be executed.

I created a simple Codeunit for everyone interested to verify these shortcut AND evaluations:

OBJECT Codeunit 50000 Shortcut AND Evaluation Test
{
  OBJECT-PROPERTIES
  {
    Date=10.11.17;
    Time=21:06:47;
    Modified=Yes;
    Version List=;
  }
  PROPERTIES
  {
    OnRun=VAR
            T@1000000000 : Integer;
            F@1000000001 : Integer;
          BEGIN
            T := 0;
            F := 0;
            CASE FALSE OF
              IsTrue(T),
              IsTrue(T),
              IsFalse(F),
              IsTrue(T):
                MESSAGE('CASE\Path taken (IF): ELSE\T: %1\F: %2',T,F);
              ELSE
                MESSAGE('CASE\Path taken (IF): THEN\T: %1\F: %2',T,F);
            END;

            T := 0;
            F := 0;
            IF FALSE IN [IsFalse(F),IsTrue(T)] THEN
              MESSAGE('IN\Path taken: ELSE\T: %1\F: %2',T,F)
            ELSE
              MESSAGE('IN\Path taken: THEN\T: %1\F: %2',T,F);
          END;

  }
  CODE
  {

    PROCEDURE IsTrue@1000000000(VAR I@1000000000 : Integer) : Boolean;
    BEGIN
      I := I + 1;
      EXIT(TRUE);
    END;

    PROCEDURE IsFalse@1000000001(VAR I@1000000000 : Integer) : Boolean;
    BEGIN
      I := I + 1;
      EXIT(FALSE);
    END;

    BEGIN
    END.
  }
}

Slawek_Guzek

vaprog wrote: »

That is actually not how this is evaluated.

I have no other option left but admit being wrong

ta5

Thanks again!