I had one job to do

Question

The code I am working with:

foreach(var currentJob in ttJobHead)
{
    foreach (var hdCase in Db.HDCase.Where(hdCase => hdCase.HDCaseNum == currentJob.HDCaseNum))
    {
        currentJob.ProjectID = hdCase.ProjectID;
    }
}

equivalent to the following pseudo code:

foreach(var t1Row in Table1)
{
    foreach (var t2Row in Table2.Where(r => r.Property1 == t1Row.Property1))
    {
        t1Row.Property2 = t2Row.Poperty2;
    }
}

I know for sure that the Table1 will only ever have 1 row, therefore the outer foreach will only ever iterate once.

My question is is there any advantage storing reference to the t1Row in a variable rather having a foreach loop?

Answer 1

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.

Answer 2

My question is is there any advantage storing reference to the t1Row in a variable rather having a foreach loop?

Not really, but you would gain a lot by removing the inner loop.

The inner loop can be removed like

foreach(var currentJob in ttJobHead)
{
    var hdCase = Db.HDCase.Where(hdCase => hdCase.HDCaseNum == currentJob.HDCaseNum).Last();
    if (hdCase != null)
    {
        currentJob.ProjectID = hdCase.ProjectID;
    }
}  

The advantage of removing the outer loop will at maximum save you some milliseconds, but won't reduce the amount of code, because you then should add a comment, why you only use the "first" element.

But hey, why could the inner loop be removed ? Very easy, for each iteration of the loop you are overwriting the currentJob.ProjectID property.