While debugging some code, I saw this particular function for removing duplicate members in an integer vector if they come on adjacent positions.
vector<int> test;
void remove_adjacent_duplicate()
{
int size = test.size();
for(int it = 0; it < size-1; )
{
if (test[it] == test[it+1])
{
test.erase(test.begin()+it);
continue;
}
it++;
}
}
Above function used to work fine intermittently. After some tests, I got to find out that this program crashed every time a valid input (having adjacent duplicate elements) is given to this function. On debugging, the main reason came as ‘for’ loop condition of the function.
This loop has a fixed number (size), which actually denotes the vector size. But if we remove some element as per 'if' condition, vector size changes But the loop continues for previously calculated size and goes to some invalid indexes. That was the reason of segmentation_fault in the program.
The right version of this function should be as below:
PS: On exploring with people, I found that the idea behind doing this was to improve efficiency, as one didn’t have to calculate size in every iteration of the loop. For keeping that thing in mind we should decrease the value of size every time we remove something from the array while using first version of the function.vector<int> test;
void remove_adjacent_duplicate()
{
for(int it = 0; it < test.size()-1; )
{
if (test[it] == test[it+1])
{
test.erase(test.begin()+it);
continue;
}
it++;
}
}
Subscribe - To get an automatic feed of all future posts subscribe here, or to receive them via email go here and enter your email address in the box. You can also like us on facebook and follow me on Twitter @akashag1001.
Its better to use STL list if deletion is required frequently as its O(1)... in vector its O(n) due to shifting of elements...