LINQ foreach - error handling and general improvement

I'm looking for two kinds of feedback:

• Is there a better way of doing what I'm attempting to do?

• Any obvious weaknesses that could potentially cause problems?

The code in question (I've broken it into lines to prevent horizontal scroll):

items.ForEach(i => 
                i.JobOrderTypeString = Context.JobOrderTypes.Where(x => 
                    x.JobOrderTypeIdentifier == i.JobOrderTypeIdentifier.CodeListItemIdentifier)
                    .FirstOrDefault().Title);

Datatypes:

• items is a List of simple ViewModels

My worries:

• If FirstOrDefault() returns null (though very unlikely), and I'm "reading" the .Title property, will my code crash?

• I'm using Context on each iteration, which I'm suspecting is probably not the best idea.

First, the least important: Personally I don't like using the .ForEach extension method as it looks too much like LINQ query commands which are mainly for projection/filtering. I just use a straight-up foreach loop most of the time. But that's just style points.

You can simplify .Where(lambda).FirstOrDefault() to FirstOrDefault(lambda).

You're referencing context every time, but you don't have to, you're right. You could enumerate over context.JobOrderTypes and store it in memory before iterating. Chances are it's going to be more performant than hammering your db every iteration. You are pulling back an entire table into memory, though, so be aware of the implications of that.

Your code is going to crash but you can fix it either very simply:

var val = ...FirstOrDefault(x => x.JobOrderTypeIdentifier == identifier);
i.JobOrderTypeString = val != null ? val.Title : "SomeDefaultValue";

Or you can flatten to an IEnumerable of Titles first (essentially saying "Give me the first of all Titles of objects that match my criteria" instead of saying "Give me the Title of the first object that matches my criteria, if no matches then null"):

i.JobOrderTypeString = context.JobOrderTypes.Where(x => x.JobOrderTypeIdentifier == identifier).SelectMany(j => j.Title).FirstOrDefault();

I'm not an EF expert so I'm not sure if the following will generate the same SQL, but it's cleaner LINQ:

i.JobOrderTypeString = context.JobOrderTypes.SelectMany(j => j.Title).FirstOrDefault(x => x.JobOrderTypeIdentifier == identifier);

Generally code that has a hydrated collection, but then goes through and iterates through the collection and sets values on each element in the collection means that you can do it better. I don't know where this code lives, but make sure it's in a place that says "right before I write these to the database, calculate!" You could also do this computation whenever the column value on i changes or when i is instantiated.

This may be a sign that your problem could be better solved by using a stored proc or a computed column, or you need to wrap the in a "BeforeInsert"-type event.

StackExchange Code Review Q#74607, answer score: 3