Compare Collection with the information in DB using EF and LINQ I want to inform the Receivers about new contracts we have this week. After I sent the Emails, the Information about sent contracts are stored in MS SQL Database.

To avoid that someone become the same Email about contract more than one time I perform this steps:


I have the Collection with new contracts as the input
I read the information from DB using Entity Framework about sent Emails
If the Emails was already sent I add new Instance to the sent Emails Collection


After that, I remove the contracts from input collection that are already in sent collection:

public Collection&lt;Contract&gt; GiveContractsThatWereNotSent(Collection&lt;Contract&gt; newThisWeekContracts)
{
   var contractsWereSent = new Collection&lt;Contract&gt;();

   using (var accountingEntities = new AccountingEntities())
   {
      foreach (Contract newContract in newThisWeekContracts)
      {
         bool found = (from sent in accountingEntities .Tbl_SentProtocol
                       where
                          sent.CONTRACT_NO == newContract.ContractNo
                          &amp;&amp; sent.CONTRACT_NO_ALT == newContract.ContractNoAlt
                        select sent ).Any();

         if (found)
         {
            contractsWereSent.Add(newContract);
         }
      }
   }    

   foreach (Contract contract in contractsWereSent)
   {
      newThisWeekContracts.Remove(contract);
   }

   return newThisWeekContracts; 
}
Several points:


Write your code against abstractions. Collection is a specific implementation.

You don't need newThisWeekContracts to be specifically a Collection. I'd take in any IEnumerable&lt;Contract&gt; instead.
By returning a Collection, the caller can add to it, or remove from it, which probably isn't the intended behavior. I'd return an IEnumerable&lt;Contract&gt;, or, if adding/removing is ok, I'd return an IList&lt;Contract&gt; or &lt;ICollection&lt;Contract&gt; (i.e. an abstraction, not a specific implementation).

I like the descriptive naming, casing is correct and all, but the names read a little bit awkward still.


It looks like the number of queries can be reduced, haven't profiled this but I would think it consumes fewer resources; the main thing is combining .Where() with .Any() and !.Any() as needed:

public IEnumerable&lt;Contract&gt; GiveContractsThatWereNotSent(IEnumerable&lt;Contract&gt; newContracts)
{
    using (var entities = new AccountingEntities())
    {
        var sentContracts =
                   entities.Tbl_SentProtocol
                           .Where(sent =&gt; 
                                  newContracts.Any(contract =&gt; 
                                                   sent.CONTRACT_NO == contract.ContractNo
                                                &amp;&amp; sent.CONTRACT_NO_ALT == contract.ContractNoAlt))
                           .ToList();

    }

    // ...
}


That gives you sent contracts (/ contractsWereSent), but that's not what you're after. Now you need to return the contracts in newContracts (/ newThisWeekContracts) that are not in sentContacts.

What you have here:

foreach (Contract contract in contractsWereSent)
{
    newThisWeekContracts.Remove(contract);
}


Involves calling EqualityComparer&lt;T&gt;.Default on each contract, which verifies whether Contract implements IEquatable&lt;T&gt;, otherwise uses overrides of .Equals and .GetHashCode if Contract overrides them, otherwise uses reference equality... which doesn't sound very neat, since if a contract is "equal" when its ContractNo and ContractNoAlt exists in sendContracts when comparing in Tbl_SentProtocol, then I'd use the same equality comparison when I want to "remove" items from the newContracts.

In fact, I wouldn't remove anything from newContracts. Keep inputs for inputs!

How about this?

IEnumerable&lt;Contract&gt; sentContracts;
using (var entities = new AccountingEntities())
{
    sentContracts = // trying to prevent horizontal scrolling...
        entities.Tbl_SentProtocol
                .Where(sent =&gt;  
                       newContracts.Any(contract =&gt; 
                                        sent.CONTRACT_NO == contract.ContractNo
                                     &amp;&amp; sent.CONTRACT_NO_ALT == contract.ContractNoAlt))
                .ToList();  
}

return newContracts.Where(contract =&gt; !sentContracts.Any(sent =&gt;
                                        sent.CONTRACT_NO == contract.ContractNo
                                     &amp;&amp; sent.CONTRACT_NO_ALT == contract.ContractNoAlt))
                   .ToList();




One more thing, the entities should be named so as to read like normal code - entity type SentContract can be mapped to table tbl_SentProtocol, and properties ContractNumber and ContractAltNumber can map to columns CONTRACT_NO and CONTRACT_NO_ALT ;)