Is it bad practice to increment more than one variable in a for loop declaration?

Question

While coding today, I came across the following loop in C#:

for(int x = 0; x < max_x_index; x++){
    for(int y = 0; y < max_y_index; y++, count++){
        some2DarrayRepresentedBy1D[count] = ...;
    }
}

Would this be consider bad practice? Would the following be a better way to achieve the same result:

for(int x = 0; x < max_x_index; x++){
    for(int y = 0; y < max_y_index; y++){
        some2DarrayRepresentedBy1D[count] = ...;
        count++;
    }
}

Would you allow both through, for example, a code review or would you tag the first example for being obscure?

Answer 1

I would agree with the OP in that the second code block would be the best approach.

Reason - readability. In my opinion, the variable incremented in the for loop statement should only be used for keeping count of how many times we're looping. count might be set before the loop, it might also be incremented / decremented in several different loops. But in either case, count isn't used to keep track of the progress of the current loop - y is in this case.

So, I would say that it's best practice, for readability sake, to only increment the variable used to loop through the for loop in the for loop line itself.

Answer 2

Of all the answers so far, I think that Charlie74 is closest to reasoning the same way I do.

The for loop consists of three segments:

for (initialization; termination; increment) {
    statement(s)
}

it is my opinion that only things that affext the termination should be part of the increment.... In other words, if the value is not part of the termination condition then it should not be part of the increment.

In this case, the count does not affect the termination condition, and as a result it does not belong in the increment section..

For what it's worth, there are a couple of things I would do differently still:

1. you have a typos in your second example.... (missing x in x < and missing y <) [This has been fixed in the question now.]
2. I would include the count++ as part of the array assignment (which is where I think it belongs):

for(int x = 0; x < max_x_index; x++){
    for(int y = 0; y < max_y_index; y++){ 
        some2Darray[count++] = ...;
    }
}

---

Discussion on ++count vs. count++ in this context

is this one of those instances where if you change i++ to ++i it will change what is assigned in the array? could you explain that a little more ... ?

Here goes.... Yes, this is one of the situations where the the pre-increment or post-increment difference is significant. Wikipedia has a section on this.

The pre-increment operator ++count increments the count variable. The post-increment operator count++ also increments the count variable. The difference is that while both ++count and count++ are expressions, the resulting values of the expressions are different.

It is important to understand what it means that count++ is an expression. An expression is something that produces a value. In the context array[count] = 1, the count is an expression, it is something with a value. That expression could instead be a constant (array[0]), a function (array[getNextIndex()], or even a more complex expression consisting of sub-expressions and operators (array[5 - 3]).

So, count++ is an expression. It is also something that modifies the value in the count variable. Similarly, ++count is an expression, and it also modifies the value of count. The difference between them is not what they do to count, but it is the value of the expression.

With count++ the value of the expression is the value of count before it is incremented.

With ++count the value of the expression is the value of count after it is incremented.

Thus, in the example above, compare two different situations:

int count = 0;
array[count++] = 1;

vs.

int count = 0;
array[++count] = 1;

In the first example, the value of the expression count++ is 0 (the value before the increment) and as a result, the value at index 0 of the array is set to 1. I.e. it is equivalent to:

int count = 0;
array[count] = 1;
count = count +1;

in the second example, the value of the expression ++count is 1 (the value after the increment) and as a result, the value at index 1 of the array is set to 1. I.e. it is equivalent to:

int count = 0;
count = count + 1;
array[count] = 1;

It is obvious, that by using ++count we will start adding data at index 1 instead of 0, and we run the risk of an out-of-bounds operation when we get to the end of the data.

In the case of adding data to an array (like what this question is doing), it makes sense to set a counter to the 'next' slot to use, and then do a post-increment so that you 'use' the next slot, and then increment it so it is pointing to the next slot again.

A bonus of doing things this way is that, at the end, the count is also the number of slots in the array that you have used.

Serendipitously, another question came up ( Finding all indices of largest value in an array ) where one potential answer illustrates this point nicely.

Answer 3

I think that @rolfl is on the right track with

for (int x = 0; x < max_x_index; x++) {
    for (int y = 0; y < max_y_index; y++) {
        some2Darray[count++] = ...;
    }
}

A good reason for using a for-loop rather than

/* initialization */;
while (/* condition */) {
    /* body */;

    /* increment */;
}

is to give your code an immediately recognizable appearance that matches its purpose. Therefore, the initialization, condition, and increment parts should all contribute to the same purpose. You should avoid introducing "off-topic" code in the for-loop header.

Additionally, some2Darray[count++] = ... effectively conveys, in just one line, that you want to populate the array — write an element, then move on to the next.

---

Other users here have objected that the count++ might be bypassed due to some unexpected flow control in the loop body. The goal of the code appears to be producing a flat 2D array, so I doubt that there would be any break or continue involved. However, if you are still concerned about consistency, you could use an assertion.

int count = 0;
for (int x = 0; x < max_x_index; x++) {
    for (int y = 0; y < max_y_index; y++) {
        some2Darray[count++] = ...;
    }
}
Debug.Assert(count == max_x_index * max_y_index);

Alternatively, eliminate count altogether and just use x * max_y_index + y.

Answer 4

There is a correctness concern with such a change. In the example given, the transformed loop is equivalent (aside from the order in which the increments occur; I'm assuming these are both int variables, so is unlikely to matter).

However if the body of the loop can contain the continue statement, suddenly there's a very large difference between these two:

int count = 0;
for (int x = 0; x < MAX; ++x, ++count)
{
    : : :
    if (sometest())
        continue;
    : : :
}

int count = 0;
for (int x = 0; x < MAX; ++x)
{
    : : :
    if (sometest())
        continue;
    : : :
    ++count;
}

In the latter code block, count is only incremented if the loop body completes. In the former code block, count is incremented whenever starting the next loop iteration. The use case may determine which of these is correct, and the decision between these may deserve a comment to help avoid breaking it later.

Note that moving ++count to the beginning of the second example would make it somewhat more similar to the first example, but in your example would require indexing your array with count - 1. As a similar alternative if you only use the index once, you could use a construct such as some2Darray[count++] instead of either, although like the original second code block this risks misbehavior from an early continue as well, and some would consider it less readable.

Answer 5

Several people on this thread have talked about how a for loop works, but few have mentioned the semantic difference between a for loop and a while loop.

When a programmer encounters a for loop when reading code, the semantic is that the loop definitely terminates and iterates over some range of numbers or some list of elements. That's why all programmers immediately know what the following loops do, and they are very easy to mentally understand and parse.

// iterate over the integer range N...M
for(int i = N; i < M; i++)
   func(N)

// iterate over every element in array
for(int i = 0; i < array.length; i++)
   func(array[i]);

// iterate over some singly linked list
for(item = list; item != NULL item=item->Next)
   func(item);

// iterate over some doubly linked list:
for(item = listHead->Next; item != listHead; item = item->Next)
   func(item);

For this reason, both of your first two arrays are well designed, and are quick and easy to understand during a code-review. The two lists are iterating x and y over all of the items [0,0] to [max_x_index, max_y_index].

But if you poison one of the two for loops so that they no longer look normal by adding some semantically unrelated variable (count) to one of the loops, suddenly the entire loop becomes harder to mentally understand. count is not related to [x,y] iterating over the loop. It's semantically related to how many elements are in some2DArray, and moving them far from each other makes the code as a whole harder to read, and inevitably, harder to maintain and debug as well.

Instead, good practice is to modify variables close to their semantic meaning, which aids comprehension of the code. To do this, you should modify count at the point where count ought to change - not when the loop iterates, but when you modify some2DArray either via inline increment:

 some2DArray[count++] = ...

or immediately afterwards:

 some2DArray[count] = ...
 count++

So in answer to your question, it's almost never a good idea to increment multiple variables within a for loop. The for loop should deal exclusively with modifying elements that are being iterated, and leave other variables well alone. Violation of this confuses the semantic of the program, and makes it much harder to read.

Answer 6

You mentioned in a comment that ultimately the end result will be setting every member of an array up to x * y

@neontapir - because y resets every iteration of x. count will end with a value of x * y. – afuzzyllama 1 hour ago

so here is my thought, Instead of doing this:

for(int x = 0; x < max_x_index; x++){
    for(int y = 0; y < max_y_index; y++, count++){ 
    {
        some2DarrayRepresentedBy1D[count] = ...;
    }
} 

Do this

for (int i = 0; i < max_x_index * max_y_index; i++)
{
    some2DarrayRepresentedBy1D[i] = ...;
}

obviously I don't know what you are doing in ... but if you are nested in two For loops and there isn't much else going on here then I assume that you are doing the same thing every time. this looks like the answer that you may be looking for, if not you need to show more code.

Answer 7

UPDATE

Matt made a really good point, but I still believe that the two following code blocks will sit well with what Matt said in his answer.

count is incremented every iteration of the inside for loop so it is changing where it should be changed in both blocks of code.

you should not increment a variable that is not declared in the Declaration of the For loop.

that said, you could do it like this

for(int x = 0, int count = 0; x < max_x_index; x++){
    for(int y = 0; y < max_y_index; y++, count++){ 
    {
        some2DarrayRepresentedBy1D[count] = ...;
    }
}

this is fairly confusing if you didn't write the code though, and thus violates the KISS Principle and/or the POLS/POLA Principle (Principle of Least Surprise/Astonishment)

going off the Answer given by Rolfl, this would be less code to write and probably slightly cleaner code, again, you should comment this code with what you did and why you do it this way.

for(int x = 0, int count = 0; x < max_x_index; x++){
    for(int y = 0; y < max_y_index; y++){ 
    {
        some2DarrayRepresentedBy1D[count++] = ...;
    }
}

---

---

Random bit of knowledge

what you could do with this

for(int x = 0; x < max_x_index; x++){
    for(int y = 0; y < max_y_index; y++){ 
    {
        some2DarrayRepresentedBy1D[count] = ...;
        count++;
    }
}

because it the incrementers are only used to progress the loops (depending on what you are counting, [this example will give you a completely different result])

for (int x = 0, int y = 100;x < max_x_index, y > max_y_index; x++, y--){
    some2DarrayRepresentedBy1D[count] = ... ;
    count++;
}

I only give this as an example that you can use two incrementers in one for loop, but what the OP posted is not one of those instances.