3
\$\begingroup\$

Here's the query:

 using (var db = CreateContext())
        {
            // performing the check for HasBeenAdded inline here, is this only one db call?
            return db.Subjects.Select(s => new SubjectDto(s){HasBeenAdded = db.Interests.Any(x => x.SubjectId == s.SubjectId)}).ToList();
        }

Basically I create a DTO from a subject and then populate a property of that DTO (HasBeenAdded) based on whether or not that entry's foreign key exists in another table. Is this the right way to go?

\$\endgroup\$
  • 1
    \$\begingroup\$ Seems relatively simple to me. Some newline characters would spruce it up a bit. \$\endgroup\$ – ChaosPandion Sep 23 '13 at 23:30
  • 1
    \$\begingroup\$ Can you not access Interests from the Subjects object given there is a link between the two? ie. HasBeenAdded = s.Interests.Any() \$\endgroup\$ – dreza Sep 24 '13 at 1:48
  • \$\begingroup\$ To play with "LINQ to Entities" I use the "LINQPad" application. It helps me a lot to optimize my queries. \$\endgroup\$ – Alfredo Cavalcanti Jan 7 '14 at 16:58
7
\$\begingroup\$

In terms of "SQL" what you need is a "Left Outer Join". So I would suggest this:

using (var db = CreateContext())
{
    var subjectDtos = from subject in Subjects
                      join interest in Interests on subject.SubjectId equals interest.SubjectId into si
                      from interest in si.DefaultIfEmpty()
                      select new SubjectDto { Subject = subject, HasBeenAdded = interest != null };
    return subjectDtos.Distinct().ToList();
}

And here is why:

  1. I think that LINQ query is more readable in 'SQL like' format.
  2. With the join clause you retrieve only the "subjects" that have an "interest". But with potential duplications if more than one "interest" has the ID of the same "subject" (For this reason I introduced the "Distinct" in the query, you can remove if the duplicate results interest you).
  3. As a matter of consistency I suggest you rather than use the constructor, initialize the property directly.
  4. For "DefaultIfEmpty" steatment docs go here. The Left Outer Join is there.

UPDATE My first code does not work. Fixed and tested now :)

\$\endgroup\$
2
\$\begingroup\$

I would normally suggest consistent usage. In the example you provide, you use both constructor parameters as well as property initialisation.

Depending on you options to change etc. the SubjectDto I would add another construction parameter to it and pass in a hasBeenAdded variable.

When talking about improving the LINQ query, I think readability and maintainability has highest priority. Often performance optimisations degrade readability and enforces you to write some additional comments.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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