LINQ

[wilbennett]

Mark,

Here is an example of what I was mentioning yesterday (HandleQuestionCommand event handler):

List<string> accessibleScenes = scenes
   .Where(m => m.Level <= userLevel)
   .Select(m =>
    {
       if (m.ChatShortcut.IndexOf(' ') >= 0)
           return $"\"{m.ChatShortcut}\"";
       else
           return m.ChatShortcut;
    })
   .ToList();

I'm not a big fan of putting blocks of code in LINQ. I think it obscures the code. LINQ is supposed to make code simple, readable and maintainable.

Here's an alternate version:

List<string> accessibleScenes = scenes
   .Where(m => m.Level <= userLevel)
   .Select(m => m.ChatShortcut.IndexOf(' ') >= 0 ? $"\"{m.ChatShortcut}\"" : m.ChatShortcut)
   .ToList();

Of course, you cannot always use the ternary and readability is still not the best. To do a general purpose solution, pull the block out into a function:

string QuotedIfSpace(string text)
{
    return text.IndexOf(' ') >= 0
       ? $"\"{text}\""
       : text;
}

Now you can do:

List<string> accessibleScenes = scenes
   .Where(m => m.Level <= userLevel)
   .Select(m => QuotedIfSpace(m.ChatShortcut))
   .ToList();

Or:

List<string> accessibleScenes = scenes
   .Where(m => m.Level <= userLevel)
   .Select(m => m.ChatShortcut)
   .Select(QuotedIfSpace)
   .ToList();

Your LINQ is much cleaner and you now have a utility function that you will probably find uses for elsewhere. If not, you can always make it a nested function in the current method.

Personally, I think all LINQ code should be like this. Single line per function call when using many and simple checks or method calls per function. Makes it really easy to see the flow and to rearrange, add, remove, comment out individual pieces, etc...

You like?

[MillerMark]

Agreed. Excellent suggestion/contribution Wil. Thank you.