Iterator bounds checking exceeding vector size

Question

My question is different because I am not looking for a range-3v solution. Also, I was specifically asking how to fix the issue with the 2nd for a loop. I accepted the answer below my question already. For I didn't need a second for loop, They showed me how to use a single index by anding it with 1 for odd iterations. This solved my problem!

I am writing a function that will take a vector assuming it is of even length of elements; In my function, I'm creating two temp vectors from the original vector where their elements are {0,2,4,6,...} and {1,3,5,7,...} respectively. Then I am adding the corresponding indexed elements and storing the results into my results vector.

Here is my function:

void sumElementPairsFromVector(const std::vector<int>& values, std::vector<int>& result)
{
    using It = std::vector<int>::const_iterator;
    std::vector<int> temp1, temp2;

    // First lets divide the original vector into two temp vectors
    for (It it1 = values.cbegin(); it1 != values.cend(); it1 += 2)
        temp1.push_back(*it1);

    for (It it2 = values.cbegin() + 1; it2 != values.cend() ; it2 += 2)
        temp2.push_back(*it2);

    // Add each corresponding vector and store that into our results.
    for (std::size_t i = 0; i < values.size() / 2; i++)
        result[i] = temp1[i] + temp2[i];
}

Here is how I'm using it:

int main() 
{         
    std::vector<int> values{ 1,2,3,4,5,6 };
    for (auto i : values)
        std::cout << i << " ";
    std::cout << '\n';

    std::vector<int> results;

    sumElementPairsFromVector(values, results);
    for (auto i : results)
        std::cout << i << " ";
    std::cout << '\n';

    return 0;
}

The expected output should be:

1 2 3 4 5 6
3 7 11

The debug assertion is failing on this line of code from the function:

for (It it2 = values.cbegin() + 1; it2 != values.cend(); it2 += 2 )

I know what is causing the error; on the last iteration after it increments by 2 and goes to check if it2 != values.cend() it is going past the end of the vector. How do I fix this?

May you use Ranges v3? The code will be much more clear and less error-prone. — Igor R., Jun 01 '19 at 14:54
@IgorR. Well I'm trying to stay within the current standard atm. Once I get this function to work correctly, I'm planning on turning it into a template lambda function. I'm not using boost; only stl. I'm using c++17 atm. don't have access to c++20. I'm also using Visual Studio 2017 CE — Francis Cugler, Jun 01 '19 at 14:55
Possible duplicate of [Iterating over an odd (even) elements only in a range-based loop](https://stackoverflow.com/questions/54850526/iterating-over-an-odd-even-elements-only-in-a-range-based-loop) — Igor R., Jun 01 '19 at 14:58
@IgorR. Kind of a duplicate but my question is specific on how to fix the second for loop in my function. — Francis Cugler, Jun 01 '19 at 15:01
@IgorR. Okay you appear to be right, just one of the answers (not accepted one was using `range-3v` for their answer; and I didn't particularly like the answers that were given. However, my question was still specific to the conditionals of the for loop. User `JeJo` gave an excellent answer in my opinion. — Francis Cugler, Jun 02 '19 at 06:25

JeJo · Accepted Answer · 2019-06-01T15:34:08.413

I know what is causing the error; on the last iteration after it increments by 2 and goes to check if it2 != values.cend() it is going past the end of the vector. How do I fix this?

You do not need two different loops for iterate through vector values.

std::vector<int> temp1, temp2;
temp1.reserve(values.size() / 2); // reserve the memory
temp2.reserve(values.size() / 2);

for (std::size_t index = 0; index < values.size(); ++index)
{
    if (index & 1) temp2.emplace_back(values[index]); // odd index
    else temp1.emplace_back(values[index]);           // even index
}

Secondly, results did not allocate any memory at the time

result[i] = temp1[i] + temp2[i];

hence out of bound undefined behavior. You should

for (std::size_t i = 0; i < std::min(temp1.size(), temp2.size()); i++)
    result.emplace_back(temp1[i] + temp2[i]);

On the other hand, if the goal is to have a resulting vector from the sum of the consecutive pair of elements, the temp1 and temp2 are redundant. The result can be filled simply:

void sumElementPairsFromVector(const std::vector<int>& values, std::vector<int>& result)
{
    result.reserve(values.size() / 2);

    for (std::size_t index = 0; index < values.size() - 1; index += 2)
        result.emplace_back(values[index] + values[index+1]);
}

I can reserve the size in main before passing it to the function; not an issue. — Francis Cugler, Jun 01 '19 at 15:03
You'll need to do more than `reserve`, @Francis. You actually need to create the elements before you assign to them. — Lightness Races in Orbit, Jun 01 '19 at 15:07
I tried this; but the output is not correct. I'm getting `3 3 3 7 3 7 3 7 11` as my output. — Francis Cugler, Jun 01 '19 at 15:07
Never mind that was my mistake I had the 2nd for loop running within the first. I pulled it out of the for loop and now I'm getting the correct results. — Francis Cugler, Jun 01 '19 at 15:13

score 2 · Answer 2 · answered Jun 01 '19 at 16:30

Since you're not working with template functions, your function just uses a vector, you don't need iterators in my opinion.

You could just have something like

...
for (int i = 0; i < values.size(); i += 2) {
    result.push_back(values[i] + values[i + 1]);
}
...

Well, if you are sure that values.size() is always even. If that isn't the case, you could do something like

void sumElementPairsFromVector(const std::vector<int>& values, std::vector<int>& result)
{
    for (int i = 1; i < values.size(); i += 2) {
        result.push_back(values[i] + values[i - 1]);
    }
    if (values.size() % 2) {
        // whatever you want to do with the last element of an uneven vector
        result.push_back(values[values.size() - 1]);
    }
}

because if you're not sure if the size of values is always even, the first method will try to access the nth element where n is larger than the amount of elements, so you may get a garbage value.

I stated assuming that the vectors are always of even length. They should not be used outside of this context. I do appreciate the feedback! — Francis Cugler, Jun 01 '19 at 17:13

Iterator bounds checking exceeding vector size

2 Answers2