4
\$\begingroup\$

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.

\$\endgroup\$
  • 2
    \$\begingroup\$ What I can see immediately is that you could replace Count() > 0 with Any(), which is simpler and also more efficient. \$\endgroup\$ – svick Jul 11 '12 at 18:38
4
\$\begingroup\$

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;
\$\endgroup\$
  • \$\begingroup\$ The way you're using Replace() means this code will most likely work, but it's not guaranteed to work for methods with “weird” names (like PerformAsyncOperationAsync). \$\endgroup\$ – svick Jul 11 '12 at 19:33
  • \$\begingroup\$ Well other than the rare case that @svick mentioned, this could work. I'll probably just make a utility method to make sure only "Async" at the end is replaced. \$\endgroup\$ – JKor Jul 11 '12 at 22:27
6
\$\begingroup\$

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));
}
\$\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.