Should I reuse a predicate for multiple IEnumerable checks

Question

In a long running Windows Service I have a custom thread pool manager that has a loop which continuously looks through a couple lists for idle threads.

Given multiple uses of the same predicate, is there any benefit to saving off the lambda as a Func?
How might the recommendation change based on the count of threads this code is interacting with, or when the Func might need to be passed to other methods to help with the work?

while (_isRunning )
{
  ...sleep a bit to prevent thrashing
  while ( _threads.Any(t => t.IsIdle) || _autreThreads.Any(t => t.IsIdle)
  {
    var assignee = _threads.FirstOrDefault(t => t.IsIdle) ?? _autreThreads.FirstOrDefault(t => t.IsIdle);
    ...give 'em something to do 
  }
}

In place of each of the lambdas I could use the following, defined once outside for the loop, for reuse.

Func<WorkerThread, bool> isIdleCondition = t => t.IsIdle;

Answer 1

Note that t => t.IsIdle is as much a literal constant as 5 or "Hello world" is. It never changes, the compiler sees that, and thus knows that the func is a constant that doesn't need to be re-instantiated every time the method is executed.

---

How might the recommendation change based on the count of threads this code is interacting with, or when the Func might need to be passed to other methods to help with the work?

It doesn't really. Even if put into a separate property, you're only ever going to get this property, never set it, so there won't be an issue as the func will always be available and race conditions are non-existant.

---

In place of each of the lambdas I could use the following, defined once outside for the loop, for reuse.

The only reason to put this in a property if for the sake of readability and reusability. There is no meaningful difference for runtime performance.

So I would still advocate putting it into a property of its own, but not for the reasons you're asking about in your question.

Answer 2

I think you are focusing your attention in the wrong place to start with.

while ( _threads.Any(t => t.IsIdle) || _autreThreads.Any(t => t.IsIdle) )
{
    var assignee = _threads.FirstOrDefault(t => t.IsIdle) ?? _autreThreads.FirstOrDefault(t => t.IsIdle);
    ...give 'em something to do 
}

Is essentially doing double work, first to check the existance of an idle thread. _threads.Any(t => t.IsIdle) || _autreThreads.Any(t => t.IsIdle) then to get that thread _threads.FirstOrDefault(t => t.IsIdle) ?? _autreThreads.FirstOrDefault(t => t.IsIdle);

You can cut the work in half by doing this:

ThreadType assignee = null
while (null != (assignee = _threads.FirstOrDefault(t => t.IsIdle) ?? _autreThreads.FirstOrDefault(t => t.IsIdle)))
{
    ...give 'em something to do 
}

Now we find and grab the idle thread in one step.

Still there are other issues here, so lets avoid attacking the lambda, and instead ask why you have to iterate those two lists independently?

ThreadType assignee = null
while (null != (assignee = _threads.Union(_autreThreads).FirstOrDefault(t => t.IsIdle)))
{
    ...give 'em something to do 
}

Now we lazy evaluate both lists, finding the first idle thread.

You could push the lambda into a function now, but why do the compilers job?

Answer 3

It can help slightly with readability in some cases, and it can help occasionally if there is a change so you don’t have to fix two places.

But it can also occasionally lead to problems when you change one place and impact another unexpectedly.

For this example, it seems like overkill.