Skip to main content
se añadieron 1522 caracteres en el cuerpo
Origen Enlace
PaperBirdMaster
  • 48.1k
  • 6
  • 45
  • 86

Además no liberas la memoria dinámica solicitada, tu función main no tiene valor de retorno. Y, generalmente no se aconseja usar using namespace std; (consulta este hilo para saber por qué) y hay que usar std::endl con cuidado (consulta este hilo para saber por qué). ###Solución. Si quieres seguir con tu diseño actual (que al igual que akko no te lo aconsejo), deberás cambiar la firma de agregar y mostrar para recibir referencias en lugar de copias:

//        Referencia a lista ---> v
void agregar(int nuevoDato, lista &unaLista) {
//                  vvvvvvv <--- literal de puntero nulo, evita usar la macro NULL
    if (unaLista == nullptr) {
//                     vvvvvvvvvvvvvvv <--- inicializacion uniforme de datos simples
        unaLista = new nodo{nuevoDato};
        std::cout << "Nodo agregado (" << unaLista -> dato << ")\n";
//      ^^^^^ <--- espacio de nombres explicito               ^^^^^
//                                    evitamos el uso de endl --/
    } else {
        agregar(nuevoDato, unaLista -> siguiente);
    }
    std::cout << '\n'; // <-- evitamos el uso de endl
//  ^^^^^ <--- espacio de nombres explicito
}

//                 v <--- Referencia a lista
void mostrar(lista &unaLista) {
// Evitando la recursividad innecesaria
    for (auto nodo = unaLista; nodo; nodo = nodo -> siguiente)
        std::cout << nodo -> dato << '\n'; // <-- evitamos el uso de endl
//      ^^^^^ <--- espacio de nombres explicito
    std::cout << "-\n"; // <-- evitamos el uso de endl
//  ^^^^^ <--- espacio de nombres explicito
}

###Propuesta. Tal y como propone akko crea un objeto lista:

struct lista
{
    void agregar(int nuevoDato) {
        agregar(nuevoDato, raiz);
    }

    void mostrar() {
        for (auto nodo = raiz; nodo; nodo = nodo -> siguiente)
            std::cout << nodo -> dato << '\n';
        std::cout << "-\n";
    }

    ~lista() {
        do
        {
            auto temp = raiz -> siguiente;
            delete raiz;
            raiz = temp;
        } while (raiz);
    }

private:

    // nodo privado, no requerimos el nodo fuera de la lista.
    struct nodo {
        int dato;
        nodo* siguiente = nullptr;
    };

    //     Referencia a puntero ---> vv
    void agregar(int nuevoDato, nodo *&unaLista) {
        if (unaLista == nullptr) {
            unaLista = new nodo{nuevoDato};
        } else {
            agregar(nuevoDato, unaLista -> siguiente);
        }
    }

    // Raiz de la lista.
    nodo *raiz = nullptr;
};

Este objeto lista sigue los principios de encapsulamiento al prohibir el acceso a los datos privados de la lista (el tipo nodo y todos los nodos almacenados) y libera la memoria al finalizar su ciclo de vida evitando fugas de memoria.

Además tu función main no tiene valor de retorno. Y generalmente no se aconseja usar using namespace std; (consulta este hilo para saber por qué) y hay que usar std::endl con cuidado (consulta este hilo para saber por qué). ###Solución. Si quieres seguir con tu diseño actual (que al igual que akko no te lo aconsejo), deberás cambiar la firma de agregar y mostrar para recibir referencias en lugar de copias:

//        Referencia a lista ---> v
void agregar(int nuevoDato, lista &unaLista) {
//                  vvvvvvv <--- literal de puntero nulo, evita usar la macro NULL
    if (unaLista == nullptr) {
//                     vvvvvvvvvvvvvvv <--- inicializacion uniforme de datos simples
        unaLista = new nodo{nuevoDato};
        std::cout << "Nodo agregado (" << unaLista -> dato << ")\n";
//      ^^^^^ <--- espacio de nombres explicito               ^^^^^
//                                    evitamos el uso de endl --/
    } else {
        agregar(nuevoDato, unaLista -> siguiente);
    }
    std::cout << '\n'; // <-- evitamos el uso de endl
//  ^^^^^ <--- espacio de nombres explicito
}

//                 v <--- Referencia a lista
void mostrar(lista &unaLista) {
// Evitando la recursividad innecesaria
    for (auto nodo = unaLista; nodo; nodo = nodo -> siguiente)
        std::cout << nodo -> dato << '\n'; // <-- evitamos el uso de endl
//      ^^^^^ <--- espacio de nombres explicito
    std::cout << "-\n"; // <-- evitamos el uso de endl
//  ^^^^^ <--- espacio de nombres explicito
}

Además no liberas la memoria dinámica solicitada, tu función main no tiene valor de retorno, generalmente no se aconseja usar using namespace std; (consulta este hilo para saber por qué) y hay que usar std::endl con cuidado (consulta este hilo para saber por qué). ###Solución. Si quieres seguir con tu diseño actual (que al igual que akko no te lo aconsejo), deberás cambiar la firma de agregar y mostrar para recibir referencias en lugar de copias:

//        Referencia a lista ---> v
void agregar(int nuevoDato, lista &unaLista) {
//                  vvvvvvv <--- literal de puntero nulo, evita usar la macro NULL
    if (unaLista == nullptr) {
//                     vvvvvvvvvvvvvvv <--- inicializacion uniforme de datos simples
        unaLista = new nodo{nuevoDato};
        std::cout << "Nodo agregado (" << unaLista -> dato << ")\n";
//      ^^^^^ <--- espacio de nombres explicito               ^^^^^
//                                    evitamos el uso de endl --/
    } else {
        agregar(nuevoDato, unaLista -> siguiente);
    }
    std::cout << '\n'; // <-- evitamos el uso de endl
//  ^^^^^ <--- espacio de nombres explicito
}

//                 v <--- Referencia a lista
void mostrar(lista &unaLista) {
// Evitando la recursividad innecesaria
    for (auto nodo = unaLista; nodo; nodo = nodo -> siguiente)
        std::cout << nodo -> dato << '\n'; // <-- evitamos el uso de endl
//      ^^^^^ <--- espacio de nombres explicito
    std::cout << "-\n"; // <-- evitamos el uso de endl
//  ^^^^^ <--- espacio de nombres explicito
}

###Propuesta. Tal y como propone akko crea un objeto lista:

struct lista
{
    void agregar(int nuevoDato) {
        agregar(nuevoDato, raiz);
    }

    void mostrar() {
        for (auto nodo = raiz; nodo; nodo = nodo -> siguiente)
            std::cout << nodo -> dato << '\n';
        std::cout << "-\n";
    }

    ~lista() {
        do
        {
            auto temp = raiz -> siguiente;
            delete raiz;
            raiz = temp;
        } while (raiz);
    }

private:

    // nodo privado, no requerimos el nodo fuera de la lista.
    struct nodo {
        int dato;
        nodo* siguiente = nullptr;
    };

    //     Referencia a puntero ---> vv
    void agregar(int nuevoDato, nodo *&unaLista) {
        if (unaLista == nullptr) {
            unaLista = new nodo{nuevoDato};
        } else {
            agregar(nuevoDato, unaLista -> siguiente);
        }
    }

    // Raiz de la lista.
    nodo *raiz = nullptr;
};

Este objeto lista sigue los principios de encapsulamiento al prohibir el acceso a los datos privados de la lista (el tipo nodo y todos los nodos almacenados) y libera la memoria al finalizar su ciclo de vida evitando fugas de memoria.

Origen Enlace
PaperBirdMaster
  • 48.1k
  • 6
  • 45
  • 86

###Los nodos no son listas. He visto en StackOverflow en Español varias veces esta confusión, y me resulta muy curioso que tantos usuarios cometan ese error.

En el código que has facilitado creas un alias de puntero a nodo (nodo*) y dices que es una lista (lista), eso es tan erróneo como decir que un escalón es una escalera, sinceramente ¿Te parecen lo mismo?:

###Los errores de tu código. El punto anterior es importante porque ese alias incorrecto que has usado es lo que está provocando tu error, en concreto está en tu función `agregar` en que pasas un puntero a nodo (`lista`) **por copia**, dado que `agregar` recibe una copia es ésta la que se modifica dejando el original (`laLista` de la función `main`) sin modificar. Por ese motivo, cuando le pasas `laLista` a la función `mostrar` (también por copia) actúa como si la lista no hubiera recibido elementos, que es de hecho lo que ha sucedido. Tal vez sin el alias te habrías dado cuenta sin ayuda, pero ese alias oculta el tipo puntero haciendo el error menos evidente.

Además tu función main no tiene valor de retorno. Y generalmente no se aconseja usar using namespace std; (consulta este hilo para saber por qué) y hay que usar std::endl con cuidado (consulta este hilo para saber por qué). ###Solución. Si quieres seguir con tu diseño actual (que al igual que akko no te lo aconsejo), deberás cambiar la firma de agregar y mostrar para recibir referencias en lugar de copias:

//        Referencia a lista ---> v
void agregar(int nuevoDato, lista &unaLista) {
//                  vvvvvvv <--- literal de puntero nulo, evita usar la macro NULL
    if (unaLista == nullptr) {
//                     vvvvvvvvvvvvvvv <--- inicializacion uniforme de datos simples
        unaLista = new nodo{nuevoDato};
        std::cout << "Nodo agregado (" << unaLista -> dato << ")\n";
//      ^^^^^ <--- espacio de nombres explicito               ^^^^^
//                                    evitamos el uso de endl --/
    } else {
        agregar(nuevoDato, unaLista -> siguiente);
    }
    std::cout << '\n'; // <-- evitamos el uso de endl
//  ^^^^^ <--- espacio de nombres explicito
}

//                 v <--- Referencia a lista
void mostrar(lista &unaLista) {
// Evitando la recursividad innecesaria
    for (auto nodo = unaLista; nodo; nodo = nodo -> siguiente)
        std::cout << nodo -> dato << '\n'; // <-- evitamos el uso de endl
//      ^^^^^ <--- espacio de nombres explicito
    std::cout << "-\n"; // <-- evitamos el uso de endl
//  ^^^^^ <--- espacio de nombres explicito
}