I was tasked with expanding a bit of customer code. Interesting enough there are two post process BPM’s running. Which one is “better”? I change the meat on the second one as both of the custom code snippets do the same thing, well almost the same thing. Which one runs faster, or is there a third winner? Thoughts, ideas, is this a BS question… lol Thanks in advance for playing.
BPM #1
foreach (var temp in (from ttSelectedSerialNumbers_Row in ttSelectedSerialNumbers where
string.Compare(ttSelectedSerialNumbers_Row.RowMod, "A", true) == 0 ||
string.Compare(ttSelectedSerialNumbers_Row.RowMod, "U", true) == 0
select ttSelectedSerialNumbers_Row))
{
foreach (var serial = (from a in Db.SerialNo where (a.SerialNumber == temp.SerialNumber && a.PartNum == temp.PartNum) select a ))
{
serial.ShortChar01 = ipNewJobNum;
}
}
Or Code #2)
var serialnumbers = Db.SerialNo;
foreach (var temp in (from ttSelectedSerialNumbers_Row in ttSelectedSerialNumbers where
string.Compare(ttSelectedSerialNumbers_Row.RowMod, "A", true) == 0 ||
string.Compare(ttSelectedSerialNumbers_Row.RowMod, "U", true) == 0
select ttSelectedSerialNumbers_Row))
{
foreach (var serial in serialnumbers)
{
if (serial.SerialNumber == temp.SerialNumber && serial.PartNum == temp.PartNum)
{
serial.ShortChar01 = ipNewJobNum;
}
}
}
The First Option is much faster, on the second option you are looping through ALL SerialNo records on the DB while the first option only selects those records which match your incoming ttSelectedSerialNumbers. You should not have the second option at all running in a production environment, and if you MUST for some reason you should in the very least break out of the loop once the first match happens.
However there are some small adjustments that could make it a bit more efficient, for example on the first code you are looping through the result set of SerialNumbers where SerialNumber = IncomingSerialNumber, since you can’t have more than one of the same serial number on the DB there is no need to loop there.
Also, this is a bit of nit pickyness but you should not use string.Compare when trying to determine if two strings are equal. According to MSDN
The CompareTo method was designed primarily for use in sorting or alphabetizing operations. It should not be used when the primary purpose of the method call is to determine whether two strings are equivalent. To determine whether two strings are equivalent, call the Equals method
Source: String.CompareTo Method (System) | Microsoft Learn
Here’s a version of 1 that should run a bit faster / more efficient using LINQ
foreach(var x in ttSelectedSerialNumbers.Where(sn=>sn.Added() || sn.Updated()))
{
//No need for a second loop since you'll only ever have 1 serial number match a given serial number and part combo
//Also don't forget to always include Company in our queries most indexes on most tables include Company
var serial = (from s in Db.SerialNo where s.Company == x.Company && s.SerialNumber == x.SerialNumber && s.PartNum == x.PartNum select s).FirstOrDefault();
if(serial != null)
serial.ShortChar01 = ipNewJobNum;
}
Another version of the same is to do it all using Lambda Expression (this doesn’t add any efficiency it just makes the code “prettier” lambda expands onto the full LINQ query above.
LAMBDA
foreach(var x in ttSelectedSerialNumbers.Where(sn=>sn.Added() || sn.Updated()))
{
//No need for a second loop since you'll only ever have 1 serial number match a given serial number and part combo
//Also don't forget to always include Company in our queries most indexes on most tables include Company
var serial = Db.SerialNo.Where(s=> s.Company == x.Company && s.SerialNumber == x.SerialNumber && x.PartNum == s.PartNum).FirstOrDefault();
if(serial != null)
serial.ShortChar01 = ipNewJobNum;
}
I am just laughing because we have two post processes events doing almost the same thing.
It is crazy to see that the #2 function is used a lot within our system. Did something change from 9.05 to 10.0? Now I have another round of clean up to do.
The string.compare is new to me I will move away from using that when possible.
Can you say definitively that the “for each” is less efficient in the LINQ code than getting just the single record? The reason I ask is because I changed my programming style years ago in the way I approached the “single record” as a result of feedback from Epicor development.
I was told that in ABL code, the “FOR EACH” was a more efficient method than the “FIND FIRST”, even when getting a single record. This was probably close to a decade ago, so I may be a bit fuzzy on the details. But the “FOR EACH” method basically went out and just got records that matched and sent them through your loop. The “FIND FIRST” method does the same thing, but then it does a sort to get the first record to return that. So, as long as you know from your experience with the application, using a FOR EACH loop versus a FIND FIRST with an IF AVAILABLE basically will give you the same results, it’s just the FOR EACH is slightly more efficient.
I’m wondering if the same logic applies here in E10. Does the “for each” simply run a query and return the results, where the “FirstOrDetault” runs the same exact query but then has to add logic to see if it returned more than one data value, and if so has to sort to return only the correct row?
If the “FirstOrDefault” is truly more efficient, then I have to change much of my code in upgrades, as I used the “For Each” methodology in much of my coding when I knew there was one and only one record.
By the way, thanks for the code snippets!! I realize in many cases we’re talking about portions of milliseconds difference, but I’m a firm believer in squeezing every bit of performance out of the code as possible.
I would use for or foreach when iterating over a whole record set. Like Jose’s first foreach. Then when grabbing only one record I would use a linq statement like FirstOrDefault.
Jose, would making the linq query like,
var serial = Db.SerialNo.FirstOrDefault(s=> s.Company == x.Company && s.SerialNumber == x.SerialNumber && x.PartNum == s.PartNum)
I do know that it is much better than option 2 since it only gets the record you need vs looping through every possible records in the DB.
Now in terms of the difference between Option 1 using a foreach and option 1 using a FirstOrDefault() the difference there is likely marginal…
I imagine there is some overhead from evaluating the (Default) portion… so perhaps the performance is slightly … better…
I suppose we could test it… you can also do it without looping or without FirstOrDefault just run the query address it via index[0] a direct access with no additional checking you can’t get any faster than that.
So the question is (if there is only 1 record to return…)
//Using a For Each
foreach (var serial = (from a in Db.SerialNo where (a.SerialNumber == temp.SerialNumber && a.PartNum == temp.PartNum) select a ))
{
serial.ShortChar01 = ipNewJobNum;
}
//VS using FirstOrDefault()
var serial = (from s in Db.SerialNo where s.Company == x.Company && s.SerialNumber == x.SerialNumber && s.PartNum == x.PartNum select s).FirstOrDefault();
if(serial != null)
serial.ShortChar01 = ipNewJobNum;
//VS Addressing the first result at Index [0]
var serial = (from s in Db.SerialNo where s.Company == x.Company && s.SerialNumber == x.SerialNumber && s.PartNum == x.PartNum select s)[0];
If I had to guess I would say that #3 above is likely the fastest, and between 1 and 2 is a toss up… I did remember that for each on LINQ is lazy which means the query is run at the very last second… so there could be some slight overhead there…
Things to ponder… I wonder if @Bart_Elia has any input on this?
I am not sure that you can pass a where clause to a the FirstOrDefault() Ed… but if you can… then I assume it would perform as fast as the others since it all gets turned into SQL anyways
You can place a conditional predicate inside a FirstOrDefault and it is more efficient considering it stops when the condition is made the first time as a where will return the whole set and then you will evaluate for the first record. The query may be the same but the linq overhead is what determines the efficiency.
@SimsTrak
So I verified that FirstOrDefault() runs a Select TOP(1) query… vs a standard Select * on the For Each… not sure if that makes that much of a difference since the where clause on both limits the result set to 1 record… but its worth noting.
Thinking about it though Select TOP(1) will ALWAYS stop at the first record it finds, while a SELECT * WHERE will still return 1 record if only one record is found but it has to still scan the entire Table for records matching that where clause. So from a DB standpoint I think that that FirstOrDefault() is likely faster…
And now we’ve all wasted jointly more time that we would have ever saved doing it one way or another! haha!
Agree, the difference is marginal. And not worth worrying about on MOST BPM’s as you’re likely dealing with a pretty small “tt” dataset to begin with. If you’re going to be looping through a million records, then it may be worth the concern.
So, the “TOP (1)”, since it stops, would DEFINITELY be worth it if you’re doing a select on a table and NOT using an index. I would guess that in most cases, you are using an index anyway, or maybe should be building one if you’ve got code that’s selecting data from a table based on a non-indexed field.