Location table's Bin Mandatory - OnValidate trigger

Cem_Karaer

Hello,

Here is the first lines of Bin Mandatory - OnValidate() trigger of the Location table.

IF "Bin Mandatory" AND NOT xRec."Bin Mandatory" THEN BEGIN
  Window.OPEN(Text010);
  ItemLedgEntry.SETCURRENTKEY("Item No.",Open,"Variant Code",Positive,"Location Code");
  IF ItemLedgEntry.FINDFIRST THEN
    REPEAT
      ItemLedgEntry.SETRANGE("Item No.",ItemLedgEntry."Item No.");
      ItemLedgEntry.SETRANGE(Open,TRUE);
      ItemLedgEntry.SETRANGE("Location Code",Code);
      IF ItemLedgEntry.FINDLAST THEN
        ERROR(Text009,FIELDCAPTION("Bin Mandatory"));
      ItemLedgEntry.SETRANGE(Open);
      ItemLedgEntry.SETRANGE("Location Code");
      ItemLedgEntry.FINDLAST;
      ItemLedgEntry.SETRANGE("Item No.");
    UNTIL ItemLedgEntry.NEXT = 0;
.
.
.

What is the point here? Why should it repeat all over the item ledger entries?
Say that I have a location with no posting but the item ledger entry table with millions of records. Then I want to click the Bin Mandatory field and wait. Wouldn't it be enough to check if there is any item ledger with that location and exit if there is none?

FDickschat

cemkaraer wrote:

What is the point here?

This is an example from MS for all developers how to code it in the worst possible way

Probably they did it because there is no Key "Location Code", "Open" existing and they did not want to add it. Or they simply wanted to update the dialog with item no. but forgot to code it.

Anyway, using findfirst/last in combination with a loop is simply nonsense. In this piece of code you should at least use find-/+ and a second variable for ILE. The Key is still not good.

As this is piece of code is maybe triggered once in a lifetime just ignore it. Or simply change it to something like this:

ItemLedgEntry.SETRANGE("Location Code",Code);
  ItemLedgEntry.SETRANGE(Open,TRUE);
  IF NOT ItemLedgEntry.ISEMPTY THEN
    ERROR(Text009,FIELDCAPTION("Bin Mandatory"));