Convert if-else within a foreach loop to LINQ

Question

What I am trying to achieve is converting this foreach loop to a LINQ equivalent. The current code loops through a list of generic objects IE List which are DB objects and then creates local objects based off of the object Type.

 foreach (var message in messages)
 {
     if (message is TextMessageData)
     {
         output.Add(new TextMessage
         {
             Content = ((TextMessageData)message).Content,
             Sender = ((TextMessageData)message).Sender,
             TimeSent = ((TextMessageData)message).TimeSent
         });
     }
     else if(message is ImageDataRecord)
     {
         output.Add(new ImageMessage
         {
             ImageUrl = ((ImageDataRecord)message).Url,
             Sender = ((ImageDataRecord)message).Sender,
             TimeSent = ((ImageDataRecord)message).TimeSent
         });
     }
 }

Thus far my code looks like this, however, I am unsure of how to return a result set for each if statement condition

var data = from message in messages
           where message is TextMessageData
           select new TextMessage
           {
               Content = ((TextMessageData)message).Content,
               Sender = ((TextMessageData)message).Sender,
               TimeSent = ((TextMessageData)message).TimeSent
           };

You can do two queries and union, but it will probably complain about the classes not being the same so you will also need to cast to the base class. What do you hope to gain by converting this to LINQ? — Paul Abbott
– Paul Abbott, Commented Aug 10, 2018 at 16:45
Both classes inherit from a base class of message so that may work yeah. I am aiming at trying to refactor the code as less code always = better :D — Chad Bonthuys
– Chad Bonthuys, Commented Aug 10, 2018 at 16:49

Simon · Accepted Answer · 2018-08-10 17:28:52Z

How about something along these lines:

var textMessages = messages.OfType<TextMessageData>().Select(m => (Message)new TextMessage() { … });
var imageMessages = messages.OfType<ImageDataRecord>().Select(m => (Message)new ImageMessage() { … });
var output = textMessages.Concat(imageMessages).ToList();

In a single line, this would look like this:

var output = messages.OfType<TextMessageData>()
                     .Select(m => (Message)new TextMessage() { … })
                     .Concat(messages.OfType<ImageDataRecord>()
                                     .Select(m => new ImageMessage() { … }))
                     .ToList();

Another solution is to "cheat" a bit and put the whole if-statement into a lambda, like this:

var output = messages.Where(message => message is TextMessageData || message is ImageDataRecord)
                     .Select(message =>
                     {
                         if (message is TextMessageData)
                         {
                             return (Message)new TextMessage
                             {
                                 …
                             };
                         }
                         else if (message is ImageDataRecord)
                         {
                             return new ImageMessage
                             {
                                 …
                             };
                         }
                         else
                             throw new Exception();
                     }).ToList();

Calling .ToList() at the end of the LINQ statement to create an actual List<T> makes only sense if you have enough memory to keep all elements in memory AND if you access the elements multiple times.

There are many other ways to reduce the amount code. For example, you might consider pattern matching, deconstructing, or the yield keyword:

IEnumerable<Message> ConvertMessages()
{
    foreach (var message in messages)
        if(message is TextMessageData tmd)
            yield return new TextMessage()
            {
                ImageUrl = tmd.Url,
                Sender = tmd.Sender,
                TimeSent = tmd.TimeSent
            }
        else if (message is ImageDataRecord ird)
            yield return new ImageMessage()
            {
                ImageUrl = ird.Url,
                Sender = ird.Sender,
                TimeSent = ird.TimeSent
            }
}

A much more important question is whether you have to do the conversion in the first place. Converting objects from one type to another type has little value by itself. It wastes time and memory.

Collectives™ on Stack Overflow

Convert if-else within a foreach loop to LINQ

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related