Help with refactoring my while select loop (code review)

Finance and Operations/AX Developer Forum
1

I had to solve a problem with an AX enhancement developed by our company. We use transfer orders to ship products to our other sites. We require quality orders to be created when the other site receives this transfer order, because of the effect that weather/transportation can have on our products. We added this ability via InventQualityReferenceType, etc. The bug I am fixing is when attempting to find the Inventory Dimension Id for the item I am receiving, there could be more than one batch per line item. Meaning, the requested qty was fulfilled by multiple INVENTDIMID’s. To minimize code changes, I am just looking for the first InventTrans for my TransOrigin that doesn’t have a quality order created yet. Not 100% fool proof, but is accurate enough for us.

I am not a ‘while select’ expert. In fact, I am new to X++. Is there a better way to do what I am doing below? {Find first InventTrans record that doesn’t have a quality order, given a InventTransOrigin record.}

while select * from inventTrans
where inventTrans.InventTransOrigin == transOrigin.RecId
{
select firstOnly inventQualityOrder where inventQualityOrder.InventDimId == inventTrans.inventDimId;
if (!inventQualityOrder)
{
inventDimId = inventTrans.inventDimId;
transRecId = inventTrans.RecId;
break;
}
}

Thanks for your suggestions in advance.

Rick

2

For reference, this is your original code:

while select * from inventTrans
    where inventTrans.InventTransOrigin == transOrigin.RecId
{
    select firstOnly inventQualityOrder
        where inventQualityOrder.InventDimId == inventTrans.inventDimId;
        
    if (!inventQualityOrder)
    {
        inventDimId = inventTrans.inventDimId;
        transRecId = inventTrans.RecId;
        break;
    }
}

The biggest problem is that you run additional database queries inside the loop, therefore you may end up with n+1 calls to database (where n is the number of related quality orders), although you’re looking for a single record only. Run just a single query (with a join), returning either one or no record. Notice that you don’t need any loop anymore, therefore you don’t have to be a ‘while select’ expert. :wink: In fact, using a while select if you expect just a single record is always suspicious (although not always wrong, because some logic can’t be put directly into queries).

Not only that you potentially fetch many records that you don’t need, but you also get many fields from database without reason. Use a field list to return only what you need.

You’ll get something like this:

select firstOnly inventDimId, RecId from inventTrans
    where inventTrans.InventTransOrigin == transOrigin.RecId
	
    notexists join inventQualityOrder
        where inventQualityOrder.InventDimId == inventTrans.inventDimId;
		
if (inventTrans.RecId)
{
    inventDimId = inventTrans.inventDimId;
    transRecId = inventTrans.RecId;
}
else { ... }}

Please note that I reviewed the code only, not the business logic.

3

That is what I attempted at first, a single qry with a notexists join, but I couldn’t figure it out. I will give it another attempt with your suggestions. Thanks.

4

Martin,

I am having the same problem that I had when I tried to implement a notexists join. It seems that the inventTrans from the main select is set to inventTrans.Recid = 0 no matter if I have a quality order or not. I am not that familiar with the notexists join statements, but I am familiar with the sql Where Not Exists (Select … from … where x=y). Will something like that work in X++??

5

If you use different code that I wrote, please share it. We can’t comment on unknown code.

If inventTrans.RecId is zero (and you didn’t forget to include it in the field list), it means that your query didn’t return anything. It may or may not to be the right result, depending on your data. If you think it’s wrong, you may want to decompose your query to smaller parts to identify which condition is filter out the data.

No, subqueries in this way are not supported. Please get used to consulting Select Statement Syntax; all the syntax is there.

By the way, if you want to see the SQL code you generated from your X++ query, no problem. Call something like this:

select generateOnly forceLiterals … inventTrans … ;
info(inventTrans.getSQLStatement());
6

I will definitely include my working code here and close the question.

And thanks for the helpful tips. Only been doing this for about 3 months, coming from C#. I learn something new everyday.

7

Martin,

Your solution worked, I just had to add some more business logic to match against InventRefId.

Thanks for your help.