correct way to resort a list

[Seven]

I need to sort a list of items and cannot seem to get it right.
what is the correct way of removing an object from a list and then reinserting into the back of the list?

Code: Select all

 
        void CSGUI_Window::popToBack(CSGUI_Window* w)
        {
            list<CSGUI_Window*>::Iterator it;
            for (it = CSChildren.begin(); it != CSChildren.end(); it++)
            {
                if ((*it) == w)
                {
                    CSChildren.erase(it);
                    CSChildren.push_back(w);
                }
            }
        }
 

or

Code: Select all

 
        void CSGUI_Window::popToBack(CSGUI_Window* w)
        {
            if (!CSChildren.empty())
            {
                list<CSGUI_Window*>::Iterator it;
                for (it = CSChildren.begin(); it != CSChildren.end(); it++)
                {
                    if ((*it) == w)
                    {
                        it = CSChildren.erase(it);
                    }
                }
            }
            CSChildren.push_back(w);
        }
 

[AReichl]

Maybe FIRST pus_back and THEN erase from list.

[CuteAlien]

Can your 'w' be more than once inside the list? If so - do you want to move _each_ to the back or only the _first_? Depending on that answer you should quit the loop after you found your first element. Also depending on that your first or your second solution are better.

Also don't use the ++iterator in the for-loop like you did. Because it will be wrong the moment you call erase. Instead when you erase in lists (or in any iterator based containters) the loops always look something like that:

Code: Select all

 
for (it = container.begin(); it != container.end(); )
{
    if ( found )
        it = container.erase(element);
    else
       it++;
}
 

And some note - don't call it popToBack but something like move to back. The word 'pop' is generally used in programming as opposite to 'push' and means you remove an element.

Also - I wouldn't even write that function at all. Just let users call 2 functions in a row here. One to erase the element and another to push the element in the back. Or... just one function to sort stuff. If you switch to using std::list that already comes with a sort function. I recommend using standard library classes in your own code - they are generally better than the Irrlicht core classes.

[Seven]

CuteAlien, your suggestion took care of the issue.

btw, you do a bangup job on here helping us out. It is greatly appreciated.

[lumirion]

there is also the option to move elements the way swap sorts do. Where iterator it1 is the element you want to move and it2 is the element at the location you want it1 be.

Code: Select all

 
ElementType temp = it1;
it1 = it2;
it2 = temp