Is looping a cycle like this bad practice?

Question

So, for this example, let's say I have a std :: vector called the original, and I want to split it in half into two different vectors. Suppose the original has an even number of elements.

std::vector<int> firstHalf; std::vector<int> secondHalf; for (int i = 0, j = original.size()/2; i < original.size() / 2; i++, j++) { firstHalf.push_back(original[i]); secondHalf.push_back(original[j]); } 

A more obvious way to do this would be to have two separate loops: one to populate firstHalf and one to populate secondHalf.

I am writing a for loop, as I considered bad practice? From my testing, this solution is slightly more efficient than having two separate loops.

Answer 1

I would say that this is not a bad practice, but it is also not a very good practice.

As Jett's answer pointed out, this can be simplified to

 std::vector<int> firstHalf(original.begin(), original.begin() + original.size() / 2); std::vector<int> secondHalf(original.begin() + original.size() / 2, original.end()); 

I would probably try to avoid recalculating original.size()/2 .

 std::size_t halfsize = original.size()/2; std::vector<int> firstHalf(original.begin(), original.begin() + halfsize); std::vector<int> secondHalf(original.begin() + halfsize, original.end()); 

or even,

 std::vector<int>::const_iterator halfway = original.begin() + original.size()/2; std::vector<int> firstHalf(original.begin(), halfway); std::vector<int> secondHalf(halfway, original.end()); 

(In C ++ 11 and later, halfsize and halfway declarations can use auto to determine type).

Whether they are better or not (e.g. readability) is very subjective.

It is significant that it is recommended to use standard algorithms in which the result is cleaner code and there is obvious equivalence. Adding an extra variable to avoid duplicate expressions can help in readability.

If you really have to use loops for some reason (for example, you do more than just copy parts of a vector to other vectors), then consider:

• use reserve() before multiple calls to push_back()
• use iterators for vector, not for text signature
• precompute reusable values ​​(for example, std::size_t halfsize = original.size()/2 before the loop, and not repeatedly calculate original.size()/2 inside the loop). This is especially true if original not const , because - depending on what your loop does - the compiler is less likely to determine that it does not change in size.
• Use a standard algorithm inside a loop, rather than inject a nested loop.

Answer 2

In fact, you can reduce your code to two lines:

 std::vector<int> firstHalf(original.begin(), original.begin() + original.size() / 2); std::vector<int> secondHalf(original.begin() + original.size() / 2, original.end()); 

Cause:

push_back can reallocate memory as the number of items increases. stl would allocate enough memory once at the beginning.

Answer 3

I would do it in two separate cycles. Thus, it also works if the number of elements is not even, and the loops are simple.

 std::vector<int> firstHalf; std::vector<int> secondHalf; size_t middle = original.size()/2; for (size_t i = 0; i < middle; i++) { firstHalf.push_back(original[i]); } for (size_t i = middle; i < original.size(); i++) { secondHalf.push_back(original[i]); } 

But I would not allow your source code to work poorly.

Answer 4

I would use two loops for the convenience of cache through spatial locality. In your source code, you jump back and forth between sections of the source array with numbers located at half the size of the array. It is much better to access array elements in stride-1 pattern. In addition, you may need to reserve space for your sub-matrices, as well as save other variables, such as size and quantity.

 size_t size = original.size(); size_t mid_size = size / 2; std::vector<int> firstHalf(mid_size); std::vector<int> secondHalf((size - mid_size == mid_size) ? mid_size : mid_size + 1); size_t i = 0; for (; i < mid_size; i++) { firstHalf[i] = original[i]; } for (; i < size; i++) { secondHalf[i - mid_size] = original[i]; } 

Jett's answer is very good.

Answer 5

For simple code like this, it's easy to see what happens. However, for more advanced code (1000 lines), I believe that most people would prefer it split into 2 for loops.

What do you mean by "more effective"? Have you looked at the assembly code?