current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite
1

Please review the two code snippets below and let me know if the second snippet is better than the first one.

In code snippet 2 I have added an inline function. According to my peers, using inline functions can help improve performance, since context switch will occur less often.

But I am not able to verify this claim. Is there any perfomance difference in the two code snippets?

Snippet 1 

foreach (var item in requestPayLoad.UserData)
{
    UserItem userItem = UserItemHelper.CreateItem(item, jobID);
    if (userItem != null)
    {
        lstUserItems.Add(userItem);
    }
}

Snippet 2

var GetUserItem = new Func<UserItem , UserJob , UserItemStruct>((custItemStruct, syncJob) =>
                  {
                      return UserItemHelper.CreateItem(custItemStruct, syncJob);
                  });

foreach (var item in requestPayLoad.UserData)
{
    UserItem userItem = GetUserItem(item,job);
    if (userItem != null)
    {
        lstUserItems.Add(userItem);
    }
}
share|improve this question
2  
Have you tried profiling these code snippets to find any performance differences? –  dreza Jul 2 '12 at 8:11
1  
The jit compiler will inline for you when it can. Don't write your code in such a way to hinder its functionality. –  Jeff Mercado Jul 2 '12 at 9:09

3 Answers 3

up vote 10 down vote accepted

You seem to be confusing several concepts, some completely unrelated.

First, the second snippet is less readable, because it adds quite a lot of complexity that's not necessary. Unless you have a good reason to do something like this (for example, significant measurable performance improvements), don't do it.

Now to the confusions: what you're doing could be called an inline function. But the performance improvements your peers were most likely talking about are from inlining functions. Inlining functions is an optimization made by the jitter, which sometimes inlines the machine code of one function into another function. This optimization is completely unrelated to creating a local delegate from lambda. And actually, by doing this you are preventing that optimization. Also, invoking a delegate has some small cost, so your second snippet will be actually slower.

To sum up, the second snippet is less readable and less efficient: I don't see any reason why should you ever write code like that. In any case, trying to optimize code like this is pointless: optimize only the code that's worth optimizing (identified by profiling) and then use empirical data to see which version is actually faster. Don't guess, measure.

You also seem to be confused about what context switch is. Context switch happens when your CPU is executing code from one thread, and then switches to executing code from another thread. This is completely unrelated to your question.

share|improve this answer
up vote 10 down vote

For what it's worth, you could re-write this as 

lstUserItems.AddRange(requestPayLoad.UserData
    .Select(item => UserItemHelper.CreateItem(item, jobID))
    .Where(userItem => userItem != null));
share|improve this answer
up vote 0 down vote

The second code snippet is a really dirty one becouse you loose the control over your code (reuseability, unit test (for this you should rethink the concept of UserItemHelper), readability).

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.