Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
Job -> Many Quotes 
Quote -> EmailLogList
EmailLogList -> Many EmailLogs

I want to return a CreatedDate from an EmailLog which has been created most recently - using a Job.Guid.

Is this the best way possible?

public DateTime? GetMostRecentEmailLog(Guid jobGuid)
{
    using (DbContext dbContext = new DbContext())
    {
        IEnumerable<Guid?> logLists = dbContext
            .Jobs
            .Include(j => j.Quotes)
            .First(j => j.Guid == jobGuid)
            .Quotes
            .Where(q => q.EmailLogListGuid != null)             
            .Select(q => q.EmailLogListGuid);

        return dbContext
            .EmailLogs
            .OrderByDescending(e => e.CreatedDate)                  
            .Where(e => logLists.Contains(e.EmailLogListGuid))
            .Select(e => e.CreatedDate)
            .FirstOrDefault();                              
    }
}
share|improve this question
up vote 3 down vote accepted

This frequently used pattern...

entities.First().Property

... is hardly ever an economical way to do querying. It fetches a complete entity from the database of which subsequently all but one property is discarded. Or, when Property is a lazy loaded collection, everything is discarded.

In general a better way is...

entities.Where(e => e.Id == id).Select(e => e.Property).First()

... because this only gets the single property from the database.

Or, if Property is a collection,...

entities.Where(e => e.Id == id)
        .SelectMany(e => e.Property)
        .Select(p => p.Property1)

Applying this would turn the first statement into...

IEnumerable<Guid> logLists = dbContext.Jobs
                                      .Where(j => j.Guid == jobGuid)
                                      .SelectMany(j => j.Quotes)
                                      .Where(q => q.EmailLogListGuid != null)             
                                      .Select(q => q.EmailLogListGuid.Value);

The second part is OK. Getting the most recent item from a collection always requires some ordering that defines "most recent".

Note that using the improved form, EF will translate this into one query, because logList isn't materialized yet. It is an expression tree1 that can be merged with the second expression. If you want, you can create one statement in LINQ as well:

return dbContext.Jobs
                .Where(j => j.Guid == jobGuid)
                .SelectMany(j => j.Quotes)
                .SelectMany(q => dbContext.EmailLogs
                    .Where(e => q.EmailLogListGuid == e.EmailLogListGuid))
                .OrderByDescending(e => e.CreatedDate)
                .Select(e => e.CreatedDate)
                .FirstOrDefault();

Or, if Quote has a navigation property EmailLogs (recommended):

return dbContext.Jobs
                .Where(j => j.Guid == jobGuid)
                .SelectMany(j => j.Quotes)
                .SelectMany(q => EmailLogs)
                .OrderByDescending(e => e.CreatedDate)
                .Select(e => e.CreatedDate)
                .FirstOrDefault();

1 Even though you cast the expression tree to IEnumerable, it's still an IQueryable containing an expression.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.