HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpMinor

Convert two collections of different objects into a single collection

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
objectsconvertintocollectiondifferenttwosinglecollections

Problem

I am combining two lists of separate objects into a single combined list. The below code works perfectly but it is not the most elegant or efficient solution. Is there a better solution?

var articles = context.Articles.Where(a => a.Author.ID == authorID).ToList();
var reviews = context.Reviews.Where(a => a.Author.ID == authorID).ToList();

var items = articles.ConvertAll(x => new ItemViewModel(x.ID, x.Title, x.Summary, "article", x.PublishDate)).Union(reviews.ConvertAll(r => new ItemViewModel(r.ID, r.Title, r.Summary, "review", r.PublishDate))).Take(20).OrderByDescending(i => i.publishDate);

Solution

Let's start with some sensible line continuation and indentation so we can see what's actually going on here.

var articles = context.Articles.Where(a => a.Author.ID == authorID).ToList();
var reviews = context.Reviews.Where(a => a.Author.ID == authorID).ToList();

var items = articles.ConvertAll(x => new ItemViewModel(x.ID, x.Title, x.Summary, "article", x.PublishDate))
    .Union(reviews.ConvertAll(r => new ItemViewModel(r.ID, r.Title, r.Summary, "review", r.PublishDate))
    ).Take(20)
    .OrderByDescending(i => i.publishDate));


Not perfect, but better. Remember, it's not necessary to cram all those lambdas onto a single line.

Is there a reason you're calling ToList()? That forces the execution of the query. If this is hitting a database with a lot of records, that could be potentially expensive.

You could also potentially clean up the second query a bit by moving the "converts" to the the prior queries.

var articles = context.Articles.Where(a => a.Author.ID == authorID)
    .ConvertAll(x => new ItemViewModel(x.ID, x.Title, x.Summary, "article", x.PublishDate));

var reviews = context.Reviews.Where(a => a.Author.ID == authorID)
    .ConvertAll(r => new ItemViewModel(r.ID, r.Title, r.Summary, "review", r.PublishDate));

var items = articles.Union(reviews)
    .Take(20)
    .OrderByDescending(i => i.publishDate));


Which shows us a potential opportunity to extract a method to remove the duplication.

Lastly, are you sure this does what you intend it to do? You didn't tell us exactly what you're trying to accomplish, but usually I would want to sort the results prior to selecting the top x results. I can't recall off the top of my head which order of Take and OrderBy would produce the correct results, but you should definitely double check that you're getting the information that you think you are.

*You might need to work on the Union syntax a bit. I'm not sitting in front of my IDE, so I didn't try to compile it.

Code Snippets

var articles = context.Articles.Where(a => a.Author.ID == authorID).ToList();
var reviews = context.Reviews.Where(a => a.Author.ID == authorID).ToList();

var items = articles.ConvertAll(x => new ItemViewModel(x.ID, x.Title, x.Summary, "article", x.PublishDate))
    .Union(reviews.ConvertAll(r => new ItemViewModel(r.ID, r.Title, r.Summary, "review", r.PublishDate))
    ).Take(20)
    .OrderByDescending(i => i.publishDate));
var articles = context.Articles.Where(a => a.Author.ID == authorID)
    .ConvertAll(x => new ItemViewModel(x.ID, x.Title, x.Summary, "article", x.PublishDate));

var reviews = context.Reviews.Where(a => a.Author.ID == authorID)
    .ConvertAll(r => new ItemViewModel(r.ID, r.Title, r.Summary, "review", r.PublishDate));

var items = articles.Union(reviews)
    .Take(20)
    .OrderByDescending(i => i.publishDate));

Context

StackExchange Code Review Q#92217, answer score: 5

Revisions (0)

No revisions yet.