Using Linq syntax, how can I remove the select n+1 issue in this method?

Question

I have the following code in a validation module. It works the way I want it to, but it has an obvious select n+1 problem. I would like to refactor this code to make a single call to the db to perform this validation.

Is there a way I can do it using linq or do I need to perform more major surgery? I am trying to avoid sprocs at this point, but am willing to listen to arguments for it.

private void EnsureIdentifiersAreNotInUse(AddOrEditClientCommand request)
 {
 var identifiers = request.Editor.Identifiers;
 for (var i = 0; i < identifiers.Count; i++)
 {
 var identifier = identifiers[i];
 var exists = dbContext.ClientIdentifers
 .Any(x => x.Client.Id != request.Editor.ClientId 
 && x.ClientIdentifierType.Id == identifier.TypeId 
 && x.Value.Equals(identifier.Value, StringComparison.InvariantCultureIgnoreCase));
 if(exists)
 request.ModelState.AddModelError("Identifiers[{0}].Value".FormatWith(i), "System Id ({0}) is already in use".FormatWith(identifier.Value));
 }
 }

• Dan: Any reason why you are trying to avoid sprocs?
• TheCoffeeCup: Hi! Welcome to Code Review. Please only state the purpose of the code in the title.
• NotMyself: I would rather limit the number of sprocs in the system to ones that are actually needed. I have gone into too many codebases that have hundreds if not thousands of sprocs that nobody knows what they do or why they are even there. So as an experiment on this project, I am attempting to lean on EF for all data access. Then I want to profile the application performance and add sprocs for obvious hot spots in the code. This question is more about how can I leverage EF/linq to get the same effect without resorting to a sproc yet.
• NotMyself: Hi @MannyMeng, thanks for the feedback and warm welcome. I don't quite follow your request. Can you point me to some examples or wiki guides for proper title format for this exchange? I am happy to edit.
• Dan: @MannyMeng you are welcome to suggest edits to questions that will be reviewed by other members of code review if you think the question is not suitable in it's current state. I think that the question is fine and the extra paragraph adds necessary context.

Answer 1

I think this might work for you

void EnsureIdentifiersAreNotInUse(AddOrEditClientCommand request)
{
 var identifiers = request.Editor.Identifiers;
 var errors = 
 dbContext.ClientIdentifers
 .Where(ci => ci.Client.Id != request.Editor.ClientId
 && identifiers.Any(i => ci.ClientIdentifierType.Id == i.TypeId
 && ci.Value.Equals(i.Value, StringComparison.InvariantCultureIgnoreCase)));
}

UPD Ok, your identifier type is not presented at db, so the query cannot be converted to a db query.

Let's try this:

void EnsureIdentifiersAreNotInUse(AddOrEditClientCommand request)
{
 var identifiers = request.Editor.Identifiers;
 var identifiersIds = identifiers.Select(i => i.TypeId).ToList();
 var identifiersValues = identifiers.Select(i => i.Value).ToList();
 var errors = 
 dbContext.ClientIdentifers
 .Where(ci => 
 {
 if (ci.Client.Id == request.Editor.ClientId)
 {
 return false;
 }
 var indexOfId = identifiersIds.IndexOf(ci.ClientIdentifierType.Id);
 if (indexOfId == -1)
 {
 return false;
 }
 return indexOfId == identifiersValues.IndexOf(ci.Value);
 }); 
}

• NotMyself: Hi @aunh, I thought you were on to something here and it was so simple. Sadly I get the following exception: Unable to create a constant value of type 'UIWeb.Models.Clients.ClientEditorForm+Identifier'. Only primitive types or enumeration types are supported in this context.
• aush: Yeah, I see your problem. Check if the update will help. In case it won't there is one more way, but you'll have to measure performance to check if it's reasonable to use it.
• NotMyself: It looks like your update breaks the relationship between identifier type and value. The validation is attempting to make sure the combination of type and value are unique.
• aush: @NotMyself why do you think so? identifiersIds and identifiersValues are (kind of) "parallel" and later we check that indexes are same.
• NotMyself: ahh yeah I see that now. Let me give it a try and ill get back to you.

Answer 2

After some discussion below with @aush, here is my updated code. I have tried using IndexOf, Tuples and a struct, which were all good learning exercises. But sadly they all throw the NotSupportedException.

This code reduces the problem to a single query, but I feel it is hacky at best. I think NHibernate would have handled this issue way better than Entity Framework 6.

private void EnsureIdentifiersAreNotInUse(AddOrEditClientCommand request)
{
 var identifiers = request.Editor.Identifiers;
 var keys = identifiers.Select(i => i.TypeId + "~" + i.Value).ToArray();
 var inuse = from i in dbContext.ClientIdentifers
 where i.Client.Id != request.Editor.ClientId
 where keys.Contains(i.ClientIdentifierType.Id + "~" + i.Value)
 select i;
 if (!inuse.Any())
 return;
 for (var i = 0; i < identifiers.Count; i++)
 {
 var identifier = identifiers[i];
 var exists = inuse.Any(e => e.ClientIdentifierType.Id == identifier.TypeId
 && e.Value.Equals(identifier.Value));
 if (exists)
 request.ModelState.AddModelError("Identifiers[{0}].Value".FormatWith(i),
 "System Id ({0}) is already in use".FormatWith(identifier.Value));
 }
}

Also, @Dan Pantry asks why I want to avoid sprocs to solve this problem. I would rather limit the number of sprocs in the system to ones that are actually needed.

I have gone into too many codebases that have hundreds if not thousands of sprocs that nobody knows what they do or why they are even there. The sheer volume of sprocs becomes baggage that is carried by the team that can become a black box of mystery that everyone is afraid to touch.

So as an experiment on this project, I am attempting to lean on EF for all data access. Then I want to profile the application performance and add sprocs for obvious hot spots in the code.

This question is more about how can I leverage EF/linq to get the same effect without resorting to a sproc (yet..).