I would say yes, there is value in removing the foreach loop, because your code will more appropriately mirror what you're expecting to happen versus what can happen. It's unclear to someone looking at this code for the first time that you're really doing a lookup based on one value as opposed to a join.
I would go a step further than storing the reference to table 1. I would actually store the row and then query table 2 for matches, e.g.
var currentJob = ttJobHead.Single();
foreach(var hdCase in Db.HDCase.Where(hCase => hCase.HDCaseNum == currentJob.HDCaseNum))
{
hdCase.ProjectID = currentJob.ProjectID;
}
Note that Single()
aggressively throws an exception when there's not a matching row. That's a good thing in a lot of cases. It fails quickly as soon as your data model changes and also clearly states to developers "an empty table makes an exception throw. Don't support it unless you intend to change that requirement." Of course you can change what you support by changing the code to use SingleOrDefault()
, First()
, or FirstOrDefault()
.
You could also merge the assignment statement into the foreach statement but it could make tracking exceptions more difficult.
I also noticed you mentioned in a comment that you intend to only find one matching record. If that is the case then remove the second foreach loop, add Single() to the end of the LINQ expression, and deal only with that record. That way you make it abundantly clear there is a 1:1 relationship between rows in these tables, no more, no less.
ttJobHead
is a DataTable ? \$\endgroup\$