Strange code on Sales Order List page

DenSter

I think I am losing my mind. The code in these two triggers on the Sales Order List is causing a HUGE delay in one of my customers' system. I just can't figure out what this is supposed to do, especially the OnNextRecord trigger. It is doing next(1) and next(-1), seemingly going through all the records up and down, but I don't understand why it's there. When I remove the triggers, the page is empty, it just does not show any records at all. How does a list page with no code not show the records in the table?

I feel stupid for asking, but can someone explain what this code is doing? I don't need suggestions for performance improvements, I want to know about this code.

TallyHo

Not a clue what it is all for, but maybe looking into the code below helps.
ShowHeader keeps returning false, this is causing the loop isn't it?
What caused the SkipLinesWithoutVAT boolean being set to true?
Are you running the list from a role center? In my case it is false.

TallyHo

Please explain why this didn't help.. I feel stupid for being rejected this way.

DenSter

You said it yourself. The first thing in that reply is "Not a clue what it is all for" and then you proceed to not explain anything but asking more questions.

Don't get me wrong, I appreciate you taking some time to reply, but I'm looking for an explanation and that did not provide it.

DenSter

Partial solution (not an answer):

The mere presence of code and/or variables in those triggers will cause the page to not show anything. Just commenting out the code was not enough. I had to actually remove the code. Then there was the presence of a local variable that still cause the page to not show anything, so I had to remove that local variable as well.

I still don't understand how this works exactly (and how I can have 20 years of experience in NAV and never noticed this before) but at least the page is behaving like a normal page again.

TallyHo

Well if that is the way you expect people to serve you, I'm off. People who offer help ask questions. The description of your problem is just too poor for a fit tight answer from anyone. But I wish you all the luck!

TallyHo

"Then there was the presence of a local variable that still cause the page to not show anything, so I had to remove that local variable as well."


Come on, this is so 1999 😄

DenSter

I don't have a problem with your reply. I did not 'reject' it either. I don't expect anyone to 'serve' me either. Like I said before, I appreciate you taking the time to reply, but it just did not answer my question.

Nothing to get upset about. Over the years I've written thousands and thousands of replies. Some of them helped, some of them didn't. Get over yourself dude.

TallyHo

.. well how long would you have continued if your first 15 replies had been rejected the way you do now. And being explained why, the way you did afterwards. It's not anger, it's disappointment blended with a bit of malicious pleasure witnessing your incompetence in this matter.

DenSter

Ok dude, whatever. Hide behind an online persona and be like that. Very mature

TallyHo

Let's not discuss maturity here. But I value your frankness.

DenSter

So moving on...

I'm still interested in someone explaining this to me. When the page opens, it seems to traverse the entire record set A-Z and then back Z-A. but I'm not quite sure why, what's the purpose? Why does it need to go down and then up through all records?

AlexDen

Hi,

This code is related to some cashflow functionality.
if SkipLinesWithoutVAT variable is set to true then this code will hide records that have zero VAT Amount.

vaprog

Hi DenSter,

I did not analyze the code in detail. ShowHeader as a name makes little sense in regard to the code that function contains. This might be copied from some other page, and not updated properly.

Anyway, it's purpose is to filter the records in a way that is not possible with setting filters on fields.

The usual approach to this is using a temporary table in the page. This moves the delay on before the page even opens, and is only feasible at all, if there are a limited (i.e. small) number of records to show. You could load only a subset into your temp table, but this will make it necessary to fiddle with these two triggers again.

OnFindRecord and OnNextRecord both expect you to return a value. If you return the default value (FALSE for OnFindRecord and 0 for OnNextRecord), by not returning any explicit value, you are telling the system, that there ist no (more) record. Therefore the page was empty.

These trigger behave the same as other triggers regarding comments and local variables. If the trigger is completely empty, the default trigger is run. If you add anything at all, you are responsible of handling it completely yourself.

What do the triggers do when left empty?
OnFindRecord does

EXIT(FIND(Which));

OnNexRecord does

EXIT(NEXT(Steps));

DenSter

vaprog wrote: »

Hi DenSter

Hey

Thanks so much for writing that up. I'm planning to work on that project again in a few days and I will go through it again with your explanation to see if it makes sense

I think the ShowHeader function is where you could program a custom condition. It's a local though and there is no event publisher so it doesn't seem to be intended to be used for SaaS

kriki

DenSter wrote: »

Partial solution (not an answer):

The mere presence of code and/or variables in those triggers will cause the page to not show anything. Just commenting out the code was not enough. I had to actually remove the code. Then there was the presence of a local variable that still cause the page to not show anything, so I had to remove that local variable as well.

I still don't understand how this works exactly (and how I can have 20 years of experience in NAV and never noticed this before) but at least the page is behaving like a normal page again.

This exists already since NAV 1.2 or 1.3... One of the first problems I had to solve with only a month or 3 of NAV experience (talking about 1999...). A colleague put commented some code in one of those triggers and nothing worked. Until I had the idea to completely remove those comments. Quite weird, but also (luckily) quite rare problem.
And it seems Microsoft kept this logic in al. After all, it is not that weird if you think about it: you have a defined a trigger (and putting comment in the trigger in C/AL also creates the trigger because the comment needs to be saved for when you reopen the object and look at the code in the trigger), so that trigger is run instead of default trigger. And how much lines of runnable C/AL / AL code you put in is your responsibility. And that can range from 0 lines to infinite lines (in the last case, you REALLY SHOULD refactor).

jreynolds

As already mentioned, this is one method to “filter” the records on a page where it is not possible by setting filters on fields.

I always try to have a quick exit to handle situations where special filtering is not required. So OnFindRecord becomes:

IF NOT SpecialFiltering THEN
  EXIT(FIND(Which))
ELSE …

And OnNextRecord becomes:

IF NOT SpecialFiltering THEN
  EXIT(NEXT(Steps))
ELSE …

In this case the special filtering is controlled by the SkipLinesWithoutVAT global Boolean variable which is set by the SkipShowingLinesWithoutVAT function. This function is only called in the "Cash Flow Management" codeunit (841), ShowTax function. It looks like there is an action on some page to display sales orders without VAT and the developer decided to use the Sales Order List page to do this. In my opinion, it is a bad idea to hijack a heavily used page (like the Sales Order List) for a special purpose that could have a performance impact on the normal use of the page.

vaprog

jreynolds wrote: »

I always try to have a quick exit to handle situations where special filtering is not required.

Well, that is, what the code essentially does. If SkipLinesWithoutVAT is FALSE (the default case), Showheader always succeeds, the trigger succeeds, and no more, and no different database requests have been made than without the modification. So, unless SkipLinesWithoutVAT is turned on (i.e set to TRUE), no noticeable performance hit should occure.

I'd like to mention, that the code as shown in DenSter's initial post handles not the general case, where

• in OnNextRecord the absolute value of Steps can be greater than 1 (the code should scan the records, in this case, and count, how many it has found, until the count of the original Steps parameter is reached).
• in OnFindRecord, Which can be one or more of '+-=><'. Only the case where Where is '>' exactly is handled correctly.