5
\$\begingroup\$

Anyone have ideas on how to improve this code at all?

string user = User.Identity.GetUserId();

        IQueryable<Article> t =
            db.Articles.Include(a => a.Categories)
                .Include(b => b.Shared)
                .Where(s => s.Private & s.createdby == User.Identity.Name);
        IQueryable<Article> q = db.Articles.Include(a => a.Shared).Where(s => s.Shared.Any(x => x.User == user));
        IQueryable<Article> art = t.Concat(q);
        IQueryable<Category> cat = db.Categories.Where(x => art.Any(c => c.categoryID == x.Id));

Especially looking to see if anyone has a fun way of putting all this into 1 query.

\$\endgroup\$
0
5
\$\begingroup\$

The first and most obvious improvement would be better variable naming:

IQueryable<Article> privateArticlesCreatedByUser =
    db.Articles.Include(article => article.Categories)
        .Include(article => article.Shared)
        .Where(article => article.Private && article.createdby == User.Identity.Name);

IQueryable<Article> articlesSharedWithUser = 
    db.Articles.Include(article => article.Shared)
        .Where(sharedArticle => sharedArticle.Shared.Any(shareRecipient => shareRecipient.User == user));

IQueryable<Article> articlesCreatedByOrSharedWithUser = privateArticlesCreatedByUser.Concat(articlesSharedWithUser);

IQueryable<Category> categories = db.Categories
    .Where(category => articlesCreatedByOrSharedWithUser.Any(articleCategory => articleCategory.categoryID == category.Id));

Obviously these names may not perfectly align with what the data means (as they're basically educated guesses on my part), but already they're a lot easier for somebody else to understand.

I also changed your & to an &&, since it then saves an equality check sometimes.

I also notice that you are including shared articles occurs twice. This might be too trivial an improvement and it's debatable if it will improve readability at all, but you could try this:

var sharedArticles = db.Articles.Include(article => article.Shared);

IQueryable<Article> privateArticlesCreatedByUser =
    sharedArticles.Include(article => article.Categories)
        .Where(article => article.Private && article.createdby == User.Identity.Name);

IQueryable<Article> articlesSharedWithUser = sharedArticles
        .Where(sharedArticle => sharedArticle.Shared.Any(shareRecipient => shareRecipient.User == user));

IQueryable<Article> articlesCreatedByOrSharedWithUser = privateArticlesCreatedByUser.Concat(articlesSharedWithUser);

IQueryable<Category> categories = db.Categories
    .Where(category => articlesCreatedByOrSharedWithUser.Any(articleCategory => articleCategory.categoryID == category.Id));
\$\endgroup\$
1
  • \$\begingroup\$ Hi Nick, Sorry I completely changed the names of the variables, however you named them perfectly :) and your solution knocked off about 1 second from the page load :) \$\endgroup\$ Sep 30 '14 at 12:37
3
\$\begingroup\$

Instead of concatenating the two collections containing two types of articles, you could have a single collection, with the two conditions ORed:

var userArticles =
    db.Articles
        .Include(article => article.Shared)
        .Include(article => article.Categories)
        .Where(article =>
            (article.Private && article.createdby == User.Identity.Name)
            || article.Shared.Any(shareRecipient => shareRecipient.User == user));
\$\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.