2

Estoy intentando realizar lo siguiente: Tengo un listado de cuentas el cual recorro con un iterador para ver qué cuentas tienen como propietario el nombre que se recibe como parámetro en la función.

Necesito que la función devuelva las cuentas pertenecientes al nombre pasado por parámetro, y en caso de que no exista ninguna asociada a dicho nombre, devuelva NULL.

Esto es lo que he implementado:

list<CuentaBancaria*> *GestionBancos::ObtenerCuentaTitular(string nombre) {
    list<CuentaBancaria*> ::iterator it = listaCuentas.begin();
    list<CuentaBancaria*>Listaresultado;
    for (it;it != listaCuentas.end();it++){
        if (nombre.compare((*it)->getTitluar().GetNombre()) == 0) {
            Listaresultado.push_back(*it);
        }
    }
    if (!Listaresultado.empty())
        return &Listaresultado; 
    else
        return NULL;
};

EL problema es que en el main.cpp me llega la dirección de memoria correctamente de la lista resultado, pero el programa peta al intentar acceder a ésta.

Lo que he podido observar es que el listado list<CuentaBancaria*>Listaresultado; al salir de la función, parece que se borra su contenido. He quitado la declaración de esta variable dentro de la función y la he creado como atributo de la clase (No es la idea) y en este caso cuando llamo la función desde el main.cpp la variable contiene los valores esperados, el programa no falla y puedo mostrar las cuentas de un nombre concreto.

Por lo tanto, he llegado a la conclusión de que el problema que tengo está en el ámbito donde estoy declarando el listado resultante.

¿Cómo podría solucionar esto para poder cumplir con la condición de retorno que comentaba al principio?

Muchas gracias!

2 respuestas 2

2

Estás intentando devolver la dirección de memoria de la lista que has creado en la pila (stack) de la llamada a función, la cual es liberada al salir de la función y en consecuencia, tu puntero pasa a apuntar a basura.

Tienes dos formas de conseguir lo que quieres, la primera y más elegante, es recibir por referencia la lista que quieres devolver rellena con los resultados.

list<CuentaBancaria*> *GestionBancos::ObtenerCuentaTitular(const string nombre, list<CuentaBancaria*> &Listaresultado) {
    list<CuentaBancaria*> ::iterator it = listaCuentas.begin();
    for (it;it != listaCuentas.end();it++){
        if (nombre.compare((*it)->getTitluar().GetNombre()) == 0) {
            Listaresultado.push_back(*it);
        }
    }
    if (!Listaresultado.empty())
        return &Listaresultado; 
    else
        return NULL;
};

La otra, consiste en reservar la lista de forma dinámica, de manera que no se libera al salir de la función pues habita en el montón (heap). Sin embargo, de esta segunda manera, debe el usuario encargarse de liberar la memoria después para evitar memory leaks.

list<CuentaBancaria*> *GestionBancos::ObtenerCuentaTitular(string nombre) {
    list<CuentaBancaria*> ::iterator it = listaCuentas.begin();
    list<CuentaBancaria*> *Listaresultado = new list<CuentaBancaria*>();
    for (it;it != listaCuentas.end();it++){
        if (nombre.compare((*it)->getTitluar().GetNombre()) == 0) {
            Listaresultado->push_back(*it);
        }
    }
    if (!Listaresultado->empty())
        return Listaresultado; 
    else
    {
        delete Listaresultado; // Liberar en este caso nosotros.
        return NULL;
    }
};
4
  • Perfecto @LuisGP, muchas gracias!! Dado que estoy obligado a cumplir las restricciones que comentaba en el ejercicio para la cabecera de la función y en su retorno, debo optar la solución de la memoria dinámica. Muchas gracias!
    – Pipelin
    el 12 nov. 2018 a las 8:36
  • Existe otra posibilidad: devolver la lista que ha generado dentro de la función como instancia, no como puntero (tal y como ha sugerido Trauma). el 12 nov. 2018 a las 9:45
  • Si quiere poder devolver nullptr no puede devolver una copia aunque se beneficiara de move.
    – LuisGP
    el 12 nov. 2018 a las 10:03
  • Corecto @LuisGP, como bien dices, he de cumplir con las condiciones que me piden en la creación de la función: - recibe un string con el nombre a buscar - devuelve la lista de cuentas asociadas a ese nombre, en caso de que no exista ninguna, ha de devolver NULL. Por lo tanto no puedo contemplar la solución de Trauma
    – Pipelin
    el 12 nov. 2018 a las 11:01
2

Tengo poco que añadir a las excelentes respuestas de LuisGP y Trauma pues ambos aportan una solución a tu problema (personalmente prefiero la de Trauma pues usa menos punteros) así que responderé aportando una solución diferente enfocada en los patrones de diseño y consejos de C++ moderno.


No uses punteros.

Los punteros en crudo (raw pointers en inglés) son propensos a errores, como has podio comprobar. En C++ moderno se desaconseja su uso; así que evita usarlos como elementos de un contenedor y evita usarlos como valor de retorno. Si lo que te ha motivado a usar punteros a CuentaBancaria como elementos del contenedor es su coste (por ser un objeto de gran tamaño) puedes usar punteros inteligentes (en este caso te conviene el puntero compartido std::shared_ptr ):

using ptr_CuentaBancaria = std::shared_ptr<CuentaBancaria>;
using ListaCuentas = std::list<ptr_CuentaBancaria>;

Nunca devuelvas NULL.

Para empezar, el literal de puntero nulo en C++ moderno es nullptr no NULL1 y para acabar, devolver NULL es considerado propenso a errores, una mala práctica y un antipatrón de diseño. Las opciones deben ser lanzar una excepción o devolver un objeto nulo.

Entendemos como objeto nulo un objeto que es válido pero que su uso resulte inocuo para el código.

Propuesta.

Teniendo en cuenta los consejos anteriores, vamos asumir que el resultado de buscar un usuario inexistente en el listado de cuentas en lugar de devolver NULL simplemente devuelve una lista vacía:

using ptr_CuentaBancaria = std::shared_ptr<CuentaBancaria>;
using ListaCuentas = std::list<ptr_CuentaBancaria>;

ListaCuentas GestionBancos::ObtenerCuentaTitular(string nombre) {
    ListaCuentas resultado;

    for (const auto &cuenta : listaCuentas) {
        if(!nombre.compare(cuenta->getTitular().GetNombre() == 0) {
            resultado.push_back(cuenta);
        }
    }

    return resultado;
};

Puedes simplificar aún más el código con una copia condicional (std::copy_if ):

using ptr_CuentaBancaria = std::shared_ptr<CuentaBancaria>;
using ListaCuentas = std::list<ptr_CuentaBancaria>;

ListaCuentas GestionBancos::ObtenerCuentaTitular(string nombre) {
    ListaCuentas resultado;

    std::copy_if(listaCuentas.begin(), listaCuentas.end(), std::back_inserter(resultado),
    [&nombre](const ptr_CuentaBancaria &cuenta) {
        return nombre.compare(cuenta->getTitular().GetNombre() == 0;
    });

    return resultado;
};

  1. NULL es sólo una macro que, según la implementación, puede ser 0 o (void *)0.
2
  • muchas gracias por tus consejos, tomaré nota de ellos y no del ejercicio. Quería comentarte que el hecho de que haya usado punteros en el contenedor es porque la clase CuentaBancaria es una clase padre de la cual derivan dos tipos de cuentas bancarias distintas CuentaBancariaXXX y cuentaBancariaYYY, es por eso que he usado una lista que contiene punteros a la clase padre. No sé si existe otra forma de tratar esto, si sabes algo, estaría encantado de leerte y aprender de ti =)
    – Pipelin
    el 12 nov. 2018 a las 11:47
  • Puedes guardar en un shared_ptr el puntero a la clase padre, aquí tienes un ejemplo. el 12 nov. 2018 a las 14:24

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.