The problem is that after removing the element of the array by shifting all its elements one position backward, the inner loop starts from the iteration i + 1. That is why for:
{ 3, 0, 5, 5, 6 }
you will remove the first 5
{ 3, 0, x, 5, 6 }
shift the elements of the array:
{ 3, 0, 5, 6, x}
but now you should start again at the i position not i+1. Otherwise, you will skip the current second 5. Moreover, you need to adapt the SIZE every time you remove an element, namely:
int remove_occurences(int x){
int i = 0, j;
int total_elements = SIZE;
while (i < total_elements){
if (arr[i] == x){
for (j = i; j < (total_elements - 1) ;j++)
arr[j] = arr[j+1];
total_elements--; // We have one element less now
}
else
i++; // Only increment if you did not remove elements from the array
}
return total_elements;
}
In the worst-case scenario, this algorithm has a time complexity of O(N^2), with N being the size of the array. Nonetheless, there is an algorithm with linear time complexity (i.e., O(N)), as one can see here and on "Nishad C M" answer. For instance:
int remove_occurences(int value_to_remove){
int i = 0, total_elements = 0;
for (; i < SIZE; i++)
if (arr[i] != value_to_remove)
arr[total_elements++] = arr[i];
return total_elements;
}
To better understand how this algorithm works, let us look at what would happen with the following example:
int arr[SIZE] = { 3, 5, 0, 9, 5 };
and removing '5'. First iteration:
if (3 != 5)
true, then arr[0] = 3; and total_elements++;
Since the first position of the array already contains 3 it looks kind of redundant to do arr[0] = 3;, however, this will be useful when one finds the value to be removed from the array (i.e., 5).
Second iteration:
if (5 != 5)
false, here the algorithm just skips and moves to the next position of the array. Now the consequence of that is that the variable i will continue to move forward, whereas the variable total_elements will stop at the position that contained the element to be removed. So in the third iteration:
if (0 != 5)
true, then arr[total_elements] = arr[i]; which in this case is arr[1] = arr[2];, and consequently arr[1] = 0;.
Using this approach removes the need (when one finds the element to remove) to do the shift of the elements explicitly, namely:
for (j = i; j < (total_elements - 1) ;j++)
arr[j] = arr[j+1];
the shift is done rather implicitly while going through the array.
Some advice regarding your code, one has to consider the following:
- The variable
SIZE cannot be decremented;
- The array will have the same allocated space throughout the application live time, regardless if you removed an element or not.
- When one removes an element from an array using your code, one is not actually removing the element but rather shifting all the elements of the array backward, and establishing that the last position of that array will not be used.
Based on the aforementioned points, I would suggest that after having allocated the array, you should not use the constant SIZE any longer because it can easily get out of sync and lead to bugs. Instead, use a variable that will keep track of the current number of elements in the array. Naturally, this would imply changes to your code such as:
int remove_occurences(int size, int value_to_remove)
Otherwise, you cannot safely call the function remove_occurences more than once since the number of elements on the array might have decreased, but the variable SIZE did not reflect that.