current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
up vote 7 down vote favorite

I've been digging through ExpressionVisitor in .NET and I found this for loop,

for (int i = 0, n = nodes.Count; i < n; i++) 
{
    Expression node = Visit(nodes[i]);
    if (newNodes != null) 
    {
        newNodes[i] = node;
    } 
    else if (!object.ReferenceEquals(node, nodes[i])) 
    {
        newNodes = new Expression[n];
        for (int j = 0; j < i; j++) 
        {
            newNodes[j] = nodes[j];
        }
        newNodes[i] = node;
    }
}

Now is there any particular reason why is that: i = 0, n = nodes.Count; i < n?

Is there any performance gain from this that is not in i = 0; i < nodes.Count?

share|improve this question
1  
In this loop n is used too and for counting node's count only one time in the beginning and not at the each iteration i think this is better way. – S.Petrosov 7 hours ago
up vote 9 down vote accepted

It is best practice to use a variable when you use the same statement more than once.

Here, nodes.Count is used in two places as below and hence, variable was used instead of executing the same statement twice.

for (int i = 0, n = nodes.Count; i < n; i++) {

And,

newNodes = new Expression[n];

Note: As discussed in the comments, performance difference is completely negligible though.

share|improve this answer
    
While you aren't wrong, I feel like in this case the performance difference is completely negligible. – Abion47 7 hours ago
3  
@Abion47 it depends what is happening in the Count property. If counts the elements on every call it is NOT negligible, if the property returns global variable which was calculated the count on the creation of the object is negligible. For example with List<T> count you don't have a problem. – mybirthname 7 hours ago
    
@Abion47 Yes you are right :-). it's a code best practice so that if you rename the nodes, you don't need to rename the same in multiple places though Visual studio will do this for you. In case what will happen if you edit it directly in the source control such as GitHub and BitBucket. – Aruna 7 hours ago
1  
@Abion47: For most code you'd be right, but Roslyn has quite strict performance requirements. They even care about the allocation cost of an enumerator. As one of the RyuJIT team members noted recently, the .NET JIT wouldn't inline property accesses for a long time, so at the very least you pay for the method call when accessing it, which is also virtual. The compiler also can't know that nodes is immutable here so common subexpression elimination would not be performed. It's also common style throughout the Roslyn codebase, I'd presume, so might be negligible here, but matter in other places. – Joey 27 mins ago
up vote 2 down vote

I don't think it has any much performance effects in this case. But as they are using this n is this line as well,

newNodes = new Expression[n];

So,

  • Program has to get node.Count only once.
  • n need not to be assigned again.
  • Looks much cleaner.

That's all there is to it.

share|improve this answer
    
How does it have not any performance effects if it calculates node.count one time instead of calculating at each iteration of the loop??? – S.Petrosov 7 hours ago
    
Those are initial conditions, they are executed only once. It won't set i=0 for each loop. – Prajwal 7 hours ago
    
i know and said it, and as you said "I don't think it has any performance effects." but it's performance effect that count is calculating only once instead of calculating at the each iteration if the loop was written only with i = 0; i < nodes.Count – S.Petrosov 7 hours ago
    
I think its negligible. Also, I forgot to add much. – Prajwal 7 hours ago
    
It depends on how the count is being calculated. – S.Petrosov 6 hours ago

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.