Logic error using .erase (C++)

Question

So my program is just reading an input of numbers and lists them in order. As seen in the output. Without the use of the algorithm library, I sorted it and getting rid of repeated data. However, if there is a data value repeated, the last value of the vector is not printed. Am I incorrectly using the .erase?

void remove_repeated(int size, vector<int>& num_vec){
    for(int i = 0; i < num_vec.size(); i++){
        if(num_vec[i]==num_vec[i+1]){
            num_vec.erase((num_vec.begin()+i));
        }   
    }
}

Output when no value is repeated:

                 **Welcome to the HW Reading Program**
 Please, enter your HW:1-10

 Do Problems: 1, 2, 3, 4, 5, 6, 7, 8, 9,and 10

Output when a value is repeated:

                 **Welcome to the HW Reading Program**
 Please, enter your HW: 1-10,1-3

 Do Problems: 1, 2, 3, 4, 5, 6, 7, 8,and 9

That is a lot of irrelevant code you've posted. You could have just posted a simple `main` program, fill a vector with repeated hard-coded values, and simply call `remove_repeated` to see how it operates. Sorting has nothing to do with the issue you're having. — PaulMcKenzie, Sep 11 '17 at 20:25
@Borgleader A) I fixed it to the best of my ability. B) What part of my indentation is not acceptable? — O2Addict, Sep 11 '17 at 20:30
@Pouya -- [Please see this as to what a Minimal, Compilable, and Verifiable Example denotes](https://ideone.com/tW31Eg). No need to show sorts, prompts, or anything else. We see that the function is faulty from the given test data. — PaulMcKenzie, Sep 11 '17 at 20:31
An example of a MCVE for this question: https://ideone.com/XV9P3G Note how it does nothing except run `remove_repeated` on a `vector` designed to expose the error and print out the result. — user4581301, Sep 11 '17 at 20:46

score 4 · Accepted Answer · answered Sep 11 '17 at 20:25

After you erase element at index i from the vector, the next element is at index i, not at index i+1. Also you have to take care not to go out of bounds when comparing to i+1, ie your loop has to look like this:

for(int i = 0; i < num_vec.size()-1;){
    if(num_vec[i]==num_vec[i+1]){
        num_vec.erase((num_vec.begin()+i));
    } else {
        i++;
    }
}

Also you should consider to use what the standard library has to offer. A set contains only unique elements, or you can use erase+unique (see eg. here for more details).

Logic error using .erase (C++)

1 Answers1