5

The natural answer would be to dereference the iterator and get the value. However, I'm stuck at using VC++ 2010 which doesn't allow dereferencing the list iterator (or does it?) I'm confused because, at one point, I need to dereference two list iterators and compare their values using: (*it) == (*it2) The program crashes with an error, only due to this line. I'm also dereferencing the iterator in a statement: printf("%d\n", (*it)); This works perfectly fine though. So, is there any way to access an element without dereferencing or using a cliext::list.

for (it=sList.begin(); it != sList.end(); it++)
{
    for (it2=it; it2 != sList.end(); it2++)
    {
        it2++;
        if ((*it) == (*it2))
        {
           sList.erase(it, it2);
        }
        it2--;
    }
}

The error I get is:

Debug Assertion Failed

Expression: list iterator not dereferencable

Surprisingly the same code runs without a problem when compiled on DevC++ (MinGW)

Farhan
  • 51
  • 1
  • 1
  • 4
  • The code might be problematic. Is it possible for you to post your code? – Saurabh Manchanda Feb 28 '11 at 18:40
  • "VC++ 2010 which doesn't allow dereferencing the list iterator" Nonsense; of course it does. "The program crashes with an error" What is the error? What does your code look like? Your code is probably in error. – James McNellis Feb 28 '11 at 18:41
  • 1
    Looks like the question is a duplicate of [this post](http://stackoverflow.com/questions/5129491/bypassing-list-iterator-dereferencing-error-in-visual-c-2010) which was also created by Farhan. – Saurabh Manchanda Feb 28 '11 at 18:45
  • As long as the iterator is inside the valid range de-referencing it will be fine. Though if the iterator is equal to end (or heaven help us after end) then you are in undefined behavior and may potentially crash the application. – Martin York Feb 28 '11 at 19:07
  • `it2 != sList.end(); ` should be `it2+1 != sList.end();` (Or `end()-1` if lists support backwards iterators, I forget) – user470379 Feb 28 '11 at 19:46
  • @user470379 I don't believe `+` or `-` are supported for non-random-access iterators. – Mark B Feb 28 '11 at 22:48

5 Answers5

5

You can in fact dereference list iterators. If you couldn't, your comparison code wouldn't have even compiled. Most likely you're accidentally dereferencing an end iterator though rather than a valid one, causing the crash. Without more code it's hard to make further observations.

EDIT: I can't make out quite what it is you're trying to do. The code you've written erases all the elements between two equal elements. I'll assume you're actually trying to remove all the duplicate elements, and that sorting the list first for performance isn't a concern/option.

EDIT2: I saw in a comment below you really want to delete the range. Updated my code.

Then try something like this:

for (it=sList.begin(); it != sList.end(); ++it)
{
    it2 = it;
    ++it2;
    while(it2 != sList.end())
    {
        if ((*it) == (*it2))
        {
           it = it2 = sList.erase(it, it2);  // Reset it as well since it will be blown away. It'll still point to the same value it did before though.
        }
        else
            ++it2;
    }
}
Mark B
  • 95,107
  • 10
  • 109
  • 188
  • 1
    You need to increment `it2` either way (unless `it2` is already the end of the list, otherwise `it2 == sList.end()+1`). Once you do an `erase`, `it` == `it2`, and they stay equal until you `erase` the rest of the list one element at a time. – user470379 Mar 01 '11 at 01:13
1

"select" Isn’t Broken.

It is rare to find a bug in the OS or the compiler, or even a third-party product or library. The bug is most likely in the application. - from The Pragmatic Programmer

It's highly likely due to your problem, not MS. Make it sure that your iterators are not invalidated while you are using them. You could accidentally erase the element which invalidate the iterator. Check this thread: What is the lifetime and validity of C++ iterators? and Good Luck! :)

UPDATE:

As I mentioned earlier, you are invalidating your iterators by erasing them in the middle of the loop. See my code below to do it properly.

std::list<int>::iterator EraseElements(std::list<int>& sList, std::list<int>::iterator start)
{
    for (std::list<int>::iterator itor1 = start; itor1 != sList.end(); ++itor1)
    {
        std::list<int>::iterator itor2(itor1);
        ++itor2;

        for ( ; itor2 != sList.end(); ++itor2)
        {
            if ((*itor1) == (*itor2))
            {
                return sList.erase(itor1, itor2);
            }
        }
    }

    return sList.end();
}


void main()
{
    // Test
    list<int> sList;
    sList.push_back(1);

    // elements will be erased
    sList.push_back(2);
    sList.push_back(3);
    //
    sList.push_back(2);

    sList.push_back(4);
    sList.push_back(5);

    // elements will be erased
    sList.push_back(6);
    sList.push_back(7);
    //
    sList.push_back(6);


    list<int>::iterator next = sList.begin();
    while (next != sList.end())
    {
        next = EraseElements(sList, next);
    }

    // It will print 1 2 4 5 6
    for (std::list<int>::const_iterator itor = sList.begin(); itor != sList.end(); ++itor)
    {
        cout << *itor << endl;
    }
}
Community
  • 1
  • 1
young
  • 2,163
  • 12
  • 19
  • I've added the code, have a look. I didn't mean that VC++ or anything had bugs. Whats surprising is that the same code runs fine when compiled under Dev-C++. – Farhan Feb 28 '11 at 19:48
  • @Farhan It doesn't actually run fine, it just seems to have the effects you want, most likely by chance. – Mark B Feb 28 '11 at 22:43
1

Its surely your code. It has two problems as far as I can see. Checkout the comments.

for (it2=it; it2 != sList.end(); it2++)
    {
        it2++;
            // there is no guarantee that it2 will now be valid
            // it should be validated again
        if ((*it) == (*it2))
        {
            // you should not modify the list here.
            // this will invalidate your iterators by default.
           sList.erase(it, it2);
        }
        it2--;
    }
vrrathod
  • 1,230
  • 2
  • 18
  • 28
  • Indeed, I didn't realize that before. Thanks! Could you tell me how should I go about erasing the elements? – Farhan Feb 28 '11 at 20:49
  • I believe you should be able to do it with splice(). Check this out http://www.cplusplus.com/reference/stl/list/splice/ It is a constant time operation, so i would highly suggest using that. Alternately, if you have freedom to modify the structures used for list, you can add a flag to delete. – vrrathod Feb 28 '11 at 21:23
  • I'm not sure how using `splice` instead fixes the problem, as it also invalidates iterators pointing to moved values. – user470379 Feb 28 '11 at 21:41
  • My Bad! Splice() would internally call erase() so its no different. Following worked for me. for(it = list1.begin(); it != list1.end(); it++){ while(true){ it2=it; it2++; list::iterator next = find(it2, list1.end(), *it); if (next == list1.end()) { break; } else { list1.erase(it2, next); list1.erase(next); } } } – vrrathod Feb 28 '11 at 22:06
1

Try this instead:

for (it=sList.begin(); it != sList.end(); it++)
{
    for (it2=sList.end()-1; it2 != it+1; it2--)
    {
        if ((*it) == (*it2))
        {
            it = sList.erase(it, it2)-1;
            break;
        }
    }
}

This new version avoids two errors in the original version of the code. First, the code now properly handles the edge conditions of the inner for loop. In the original code, the for loop allowed it2 to go up to sList.end()-1, but then the next line incremented it to sList.end() on the last iteration. The next line then dereferenced this (invalid) iterator which is one past the last value of the list (because that's what end returns, it's not an iterator to the last value of the list).

Second, calling erase invalidates any iterators pointing to any of the values erased (which in this case would including any iterators from it to it2-1). By starting at the end of the list and working our way forward, we no longer have to continue iterating when we find the value, and can break from the inner loop once we find it. erase returns an iterator to the next element in the list after the elements deleted (which would be the next element we want to try for it). But since the for loop increments it, we subtract 1 from what's returned by erase so that it points to the right element once it's incremented at the beginning of the next loop iteration. (Note that in the case that it points to the first element, we actually temporarily set it to point an element before the beginning of the list; however, this is only temporary and we don't dereference the iterator while it's pointing outside the list).

Note that this preserves the original behavior of the code for the case 0 2 3 4 5 1 6 7 8 0 9 10 11 1. You haven't explicitly stated what order the deletes should occur (should the elements between 0's be erased first, or the elements between 1's, or do we need to add additional logic to actually erase the whole range except for the first 0 and 1?), but this code behaves like the original and erases the numbers in between the 0's and ignores the fact that the 9 10 11 afterwards was original in between matching 1's.

user470379
  • 4,879
  • 16
  • 21
0

It is really unclear what this code snippet or whatever code you get the error from is trying to do.

It appears what you want to do is for each item delete all items between it and the next matching item, or maybe it is the last matching item.

your inner loop iteration is double stepping from the loop increment and then incrementing again inside the loop.

your not checking if you have hit/passed the end of the list after doing the inner iteration which could lead to the crash when doing the comparison

after erasing you decrement it2, which then puts it before what it1 was (and is now deleted).

Greg Domjan
  • 13,943
  • 6
  • 43
  • 59
  • Yes, I'm trying to remove all elements between two copies (including one of them). Eg: if there is 0, 1, 2, 1, 3 on the list, I want to remove 1 and 2 from the list, so it becomes 0, 1, 3. What would be a better approach to deleting the elements? – Farhan Feb 28 '11 at 20:51