-1

I have to write a function that erases an element out of the list if it's bigger than the previous element.(The previous element is the one which points to the next element before deletion)

I think I've basically finished it but I don't know why it doesn't erase 5 out of my list.

void deleteBigger(list<int> s){
    list<int>::iterator test;
    for(test = s.begin(); test != s.end(); test++){
        int sk1=*test;
        cout<<sk1<<endl;
        test--;
        int sk2=*test;
        cout<<sk2<<endl;
        if(sk1>sk2){
            cout<<"Im here!\n";
            s.erase(test);
        }
        test++;
    }
}

It doesn't give an error or anything it just doesn't erase. I tried to test the erase method in the main function of the program, and there it worked fine.

richiE
  • 33
  • 3
  • 2
    https://stackoverflow.com/questions/596162/can-you-remove-elements-from-a-stdlist-while-iterating-through-it – fukanchik Jun 07 '22 at 21:32
  • 6
    `void deleteBigger(list s)` `s` is passed by value, not reference. The function is making changes to a copy. – user4581301 Jun 07 '22 at 21:32
  • 1
    In the first iteration of the loop `test` is `s.begin()` you then decrement `test` which is undefined behaviour – Alan Birtles Jun 07 '22 at 21:34
  • Expanding on the point I believe @fukanchik is making, after `s.erase(test);`, `test++;` just isn't a good idea. `test` is invalid. – user4581301 Jun 07 '22 at 21:35
  • 1
    Note that [`std::list::erase`](https://en.cppreference.com/w/cpp/container/list/erase) *returns* something. – Bob__ Jun 07 '22 at 21:41

2 Answers2

1

There are three problems with your code:

  1. Your list is passed by value, not reference. So you are changing a copy of your list and it doesn't alter the original container
  2. You try to remove an element from a list while iterating it. Edit: As @Remy Lebeau mentioned in the comments, to be more precise it's a problem because you don't update the iterator properly, but not a problem on its own. Be advised, that when you remove an element from a list, the iterator which pointed to the erased element is considered invalidated.
  3. Upon the first iteration, you decremented the iterator out of the container's bounds

Summing it up, what you might want to have here looks something like this:

void deleteBigger(std::list<int> &s) {
    using namespace std;

    if (s.empty()) {
        return;
    }

    for(auto test = next(s.cbegin()); test != s.cend(); ++test){
        while ((*test > *prev(test)) && (test != s.cend())) {
            test = s.erase(test);
        }
    }
}
The Dreams Wind
  • 8,416
  • 2
  • 19
  • 49
  • 1
    #2 is fine to do, as long as you pay attention to `erase()`'s return value properly, eg: `if (sk1 > sk2){ ... test = s.erase(test); } else { ++test; }` – Remy Lebeau Jun 07 '22 at 21:39
  • Thanks for the answer! Basically the main problem was that I didn't reference the list I was passing through. – richiE Jun 08 '22 at 06:39
-1

I've copied your code and it doesn't work. The problem is your iterator pointer "test". You can't degree a pointer at the begin. The only thing you can do is use a control.

Note: it's wrong decrement a pointer because you are decrementing of (32 bits) the index of memory. In this case there is overriding -- operator that saves your program but be careful next times

Control your program. It's important use a debugger editor where you can stop the program at certain point and control the value of the variables

Jalbo
  • 19
  • 4