Simplify a complex LINQ query

Question

Any suggestions how I could simplify this LINQ query?

from type in assembly.GetTypes()
where type.IsPublic && !type.IsSealed && type.IsClass
where (from method in type.GetMethods()
       from typeEvent in type.GetEvents()
       where method.Name.EndsWith("Async")
       where typeEvent.Name.EndsWith("Completed")
       let operationName = method.Name.Substring(0, method.Name.Length - "Async".Length)
       where typeEvent.Name == operationName + "Completed"
       select new { method, typeEvent }).Count() > 0
select type;

assembly is of type System.Reflection.Assembly. If you need more information, just ask.

Answer 1

You can do a join between the methods and events :

from type in assembly.GetTypes()
where type.IsPublic && !type.IsSealed && type.IsClass
where (from method in type.GetMethods()
        join typeEvent in type.GetEvents()
            on method.Name.Replace("Async", "Completed") equals typeEvent.Name
        select new { method, typeEvent }).Any()
select type;

Answer 2

Original code (wrapped up in a method):

public IEnumerable<Type> GetAsyncCompletableTypes(Assembly assembly)
{
    return from type in assembly.GetTypes()
           where type.IsPublic && !type.IsSealed && type.IsClass
           where (from method in type.GetMethods()
                  from typeEvent in type.GetEvents()
                  where method.Name.EndsWith("Async")
                  where typeEvent.Name.EndsWith("Completed")
                  let operationName = method.Name.Substring(0, method.Name.Length - "Async".Length)
                  where typeEvent.Name == operationName + "Completed"
                  select new { method, typeEvent }).Count() > 0
           select type;
}

The first thing I notice is that the overall structure of the query is:

• Find me all types in the assembly
• Where the type is a public, non-sealed class
• And where the type passes a complicated looking filter.

I'd split the complicated looking filter out into a method to start with:

public IEnumerable<Type> GetAsyncCompletableTypes(Assembly assembly)
{
    return from type in assembly.GetTypes()
       where type.IsPublic && !type.IsSealed && type.IsClass
       where IsAsyncCompletableType(type)
       select type;
}

private static bool IsAsyncCompletableType(Type type)
{
    return (from method in type.GetMethods()
        from typeEvent in type.GetEvents()
        where method.Name.EndsWith("Async")
        where typeEvent.Name.EndsWith("Completed")
        let operationName = method.Name.Substring(0, method.Name.Length - "Async".Length)
        where typeEvent.Name == operationName + "Completed"
        select new { method, typeEvent }).Count() > 0;
}

That gives us two simpler queries to look at. The only thing I can see in the first part is that the repeated where can be collapsed into a single one:

public IEnumerable<Type> GetAsyncCompletableTypes(Assembly assembly)
{
    return from type in assembly.GetTypes()
       where type.IsPublic && !type.IsSealed && type.IsClass && IsAsyncCompletableType(type)
       select type;
}

Onto the second part. The lines in the query seem to alternate between being related to the methods and the events - reordering the lines will make it clearer what's going on:

private static bool IsAsyncCompletableType(Type type)
{
    return (from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        let operationName = method.Name.Substring(0, method.Name.Length - "Async".Length)
        from typeEvent in type.GetEvents()
        where typeEvent.Name.EndsWith("Completed")
        where typeEvent.Name == operationName + "Completed"
        select 0).Any();
}

We're now using the method variable up to the let operation, and never using it again, so we can select the operationName in a subquery instead of using let.

private static bool IsAsyncCompletableType(Type type)
{
    var operationNames = from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        select method.Name.Substring(0, method.Name.Length - "Async".Length);

    return (from operationName in operationNames            
        from typeEvent in type.GetEvents()
        where typeEvent.Name.EndsWith("Completed")
        where typeEvent.Name == operationName + "Completed"
        select 0).Any();
}

You may notice that the two where lines don't make a lot of sense together at this point:

• Pick events
• Where the name ends with "Completed"
• And where the name starts with operationName and ends with "Completed"

The first line is redundant. So we can remove it:

private static bool IsAsyncCompletableType(Type type)
{
    var operationNames = from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        select method.Name.Substring(0, method.Name.Length - "Async".Length);

    return (from operationName in operationNames
        from typeEvent in type.GetEvents()
        where typeEvent.Name == operationName + "Completed"
        select 0).Any();
}

The only thing we ever do to operationName is add "Completed" to it - we might as well do that when we create the operationName (and rename it appropriately):

private static bool IsAsyncCompletableType(Type type)
{
    var eventNamesFromMethods = from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        select method.Name.Substring(0, method.Name.Length - "Async".Length) + "Completed";

    return (from eventNameFromMethod in eventNamesFromMethods
        from typeEvent in type.GetEvents()
        where typeEvent.Name == eventNameFromMethod
        select 0).Any();
}

We're now asking the computer to iterate over all the events and select its name for every eventNameFromMethod. We can pre-compute these and put them into a fast lookup container - a HashSet:

private static bool IsAsyncCompletableType(Type type)
{
    var eventNamesFromMethods = from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        select method.Name.Substring(0, method.Name.Length - "Async".Length) + "Completed";

    var eventNames = new HashSet<string>(
        from typeEvent in type.GetEvents()
        select typeEvent.Name);

    return (from eventNameFromMethod in eventNamesFromMethods
        where eventNames.Contains(eventNameFromMethod)
        select 0).Any();
}

That last bit is now really just a where and an Any. But Any has an overload that takes a condition, so let's use it:

private static bool IsAsyncCompletableType(Type type)
{
    var eventNamesFromMethods = from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        select method.Name.Substring(0, method.Name.Length - "Async".Length) + "Completed";

    var eventNames = new HashSet<string>(
        from typeEvent in type.GetEvents()
        select typeEvent.Name);

    return eventNamesFromMethods.Any(eventNameFromMethod => eventNames.Contains(eventNameFromMethod));
}

And with a little bit of rearranging to remove the eventNamesFromMethods variable that only gets used once:

private static bool IsAsyncCompletableType(Type type)
{
    var eventNames = new HashSet<string>(
        from typeEvent in type.GetEvents()
        select typeEvent.Name);

    return (from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        select method.Name.Substring(0, method.Name.Length - "Async".Length) + "Completed")
        .Any(eventNameFromMethod => eventNames.Contains(eventNameFromMethod));
}

Personally (and feel free to disagree here), I find the extension method syntax far easier to read and reason about (especially when you have to use things like Any anyway), so here's what it looks like using that:

public static IEnumerable<Type> GetAsyncCompletableTypes(Assembly assembly)
{
    return assembly.GetTypes()
        .Where(type => type.IsPublic && !type.IsSealed && type.IsClass && IsAsyncCompletableType(type));
}

private static bool IsAsyncCompletableType(Type type)
{
    var eventNames = new HashSet<string>(type.GetEvents().Select(typeEvent => typeEvent.Name));

    return type.GetMethods()
        .Where(method => method.Name.EndsWith("Async"))
        .Select(method => method.Name.Substring(0, method.Name.Length - "Async".Length) + "Completed")
        .Any(eventNameFromMethod => eventNames.Contains(eventNameFromMethod));
}