1

Muy buenas a tod@s :).

Tengo el siguiente código para comprobar si dos objetos de una lista de punteros a una clase Objeto están en la misma posición (x, y) y si están, borrar uno de ellos. Funciona perfectamente en Code::Blocks 17 pero en Visual Studio 17 lanza una excepción en la línea marcada con 'EXCEPCIÓN' con el mensaje:

'list iterator not incrementable'

y no entiendo por qué. Me "obligan" a hacerlo en Visual Studio por eso tengo que arreglarlo. Como siempre si alguien me puede echar un cable se lo agradeceré enormemente :).

std::list<Objeto*> objetos_;

for (auto Obj = objetos_.begin(); Obj != objetos_.end(); Obj++)
    {
        for (auto Obj2 = Obj; Obj2 != objetos_.end(); Obj2++) ---> EXCEPCIÓN
        {
             if ((*Obj)->Get_X() == (*Obj2)->Get_X() && (*Obj)->Get_Y() == (*Obj2)->Get_Y())
             {
                 if ((*Obj)->Get_Nombre() != "USUARIO")
                 {
                      delete(*Obj);
                      Obj = objetos_.erase(Obj);
                 }
                 if ((*Obj2)->Get_Nombre() != "USUARIO")
                 {
                      delete(*Obj2);
                      Obj2 = objetos_.erase(Obj2);
                 }
             }
         }
     }
14
  • 1
    Y objetos... ¿qué tipo de objeto es?
    – eferion
    Commented el 17 ene. 2018 a las 14:03
  • @eferion es una lista de punteros de una clase llamada 'objeto' (soy muy poco original, lo se ;)) que es una clase abstracta.
    – Battyhal
    Commented el 17 ene. 2018 a las 14:06
  • 1
    "una lista de punteros"... std::vector? std::list? std::set?
    – eferion
    Commented el 17 ene. 2018 a las 14:07
  • 2
    Tiene toda la pinta que has incrementado el iterador que apunta a un elemento borrado. Commented el 17 ene. 2018 a las 14:13
  • 1
    @Battyhal porque el comportamiento indefinido puede provocar fallos, puede no provocarlos o puede invocar demonios que saldrían disparados de tus fosas nasales. Commented el 17 ene. 2018 a las 14:16

3 respuestas 3

2

Tiene toda la pinta que has incrementado el iterador end.

La función list::erase borra un elemento de la lista y devuelve el elemento a continuación del borrado, pudiendo ser este elemento el final de la lista (end) y siendo el final de la lista un elemento no incrementable (de ahí la excepción).

Deberías cambiar tus bucles para tener en cuenta esta particularidad:

for (auto Obj = objetos_.begin(); Obj != objetos_.end(); /*Obj++*/)
//                             No incrementamos aqui --> ~~~~~~~~~
{
    for (auto Obj2 = Obj; Obj2 != objetos_.end(); /*Obj2++*/)
    //                 No inctrementamos aqui --> ~~~~~~~~~~
    {
         if ((*Obj)->Get_X() == (*Obj2)->Get_X() && (*Obj)->Get_Y() == (*Obj2)->Get_Y())
         {
             if ((*Obj)->Get_Nombre() != "USUARIO")
             {
                  delete(*Obj);
                  Obj = objetos_.erase(Obj);
             }
             else
             {
                 ++Obj; // Incrementamos aqui
             }
             if ((*Obj2)->Get_Nombre() != "USUARIO")
             {
                  delete(*Obj2);
                  Obj2 = objetos_.erase(Obj2);
             }
             else
             {
                 ++Obj2; // Incrementamos aqui
             }
         }
     }
 }

Pero de todas maneras, este código además de ser incómodo de leer, está lejos del estilo de C++ moderno... y ni siquiera estoy segura de que funcione pues usas dos iteradores diferentes para recorrer una misma colección. Te aconsejo cambiar el código para usar la librería <algorithm>:

auto borrar = std::remove_if(objetos_.begin(), objetos_.end(),
              [](Objeto* o)
              {
                  return o->Get_Nombre() == "USUARIO";
              });

La función std::remove_if, mueve al final del contenedor los elementos que no cumplen con la condición y devuelve un iterador al primer elemento que no la cumplió, así que ahora se pueden borrar los punteros y eliminarlos de la lista:

std::for_each(borrar, objetos_.end(), [](Objeto* o) { delete o; });
objetos_.erase(borrar, objetos_.end());
1
  • Cierto... pasé por alto esta posibilidad
    – eferion
    Commented el 17 ene. 2018 a las 14:49
1

Este algoritmo hace aguas... y el error no te está dando en visual studio sino que es dependiente del compilador... o mejor dicho, de la implementación de la librería estándar:

for (auto Obj2 = Obj; Obj2 != objetos_.end(); Obj2++)

Para empezar, en cada primera iteracción del segundo for, Obj y Obj2 (yo los llamaría It1 e It2 ya que son iteradores, no objetos), apuntan al mismo elemento... luego en esta primera iteración se accede al if sí o sí.

En este contexto... ¿Qué sucede si el primer elemento es uno de los que hay que eliminar?

if ((*Obj)->Get_Nombre() != "USUARIO")
{
  delete(*Obj);
  Obj = objetos_.erase(Obj);
}
if ((*Obj2)->Get_Nombre() != "USUARIO")
{
  delete(*Obj2);
  Obj2 = objetos_.erase(Obj2);
}

Pasa que o bien se hace erase de Obj o bien de Obj2... pero ojo, que ambos iteradores son el mismo... si haces erase de Obj entonces Obj2 se convertirá en un iterador no válido ya que el elemento al que apuntaba ya no se encuentra en la lista y lo mismo le pasará a Obj si se hace erase de Obj2:

// caso1:
Obj = objetos_.erase(Obj); // Obj2 ya no es valido

// caso2:
Obj2 = objetos_.erase(Obj2); // Obj ya no es valido

De hecho, podemos leer en la documentación de la función lo siguiente:

References and iterators to the erased elements are invalidated. Other references and iterators are not affected.

Es decir:

Las referencias e iteradores al elemento a eliminar dejarán de ser válidos. El resto de iteradores y referencias no se ven afectados.

El caso es que el catálogo de problemas que podría llegar a darte es bastante extenso... si no te falla el iterador podrías incluso tener un error por doble borrado:

  • En la iteración X se borra Obj2, con lo que Obj deja de ser válido
  • En la siguiente iteración se borra Obj... que apunta a un elemento ya borrado... premio!!!

El problema se puede reproducir de otras tantas formas... pero el resumen es que si Obj==Obj2 y haces un erase, uno de los dos iteradores va a dejar de ser un iterador válido y el algoritmo se te va a caer por algún lado.

Una posible solución:

for (auto Obj2 = std::next(Obj,1); Obj2 != objetos_.end(); Obj2++)

Así, Obj2 empezará apuntando a la siguiente posición respecto a Obj.

0

Muchas gracias por responder Paula_plus_plus y Eferion :).

Con respecto a :

este código además de ser incómodo de leer, está lejos del estilo de C++ moderno

seguro que sí pero es la forma que conocía y no se me ocurría otra. Estoy empezando ahora con las lambdas. Gracias por la sugerencia y cambiaré el código por supuesto ;).

Y sobre :

y ni siquiera estoy segura de que funcione pues usas dos iteradores diferentes para recorrer una misma colección.

te aseguro que funciona (lo puedes descargar aquí : https://github.com/battyhal/Bomberman.git)... en Code::Blocks y Zinjai que eran los que usaba, pero no en Visual Studio 17. Supongo que el compilador es más 'quisquilloso' en este último ;).

Tu Respuesta

By clicking “Publica tu respuesta”, you agree to our terms of service and acknowledge you have read our privacy policy.

¿No es la respuesta que buscas? Examina otras preguntas con la etiqueta o formula tu propia pregunta.