-1

I've written the following code for a recursive ToH algorithm with stacks and can't figure out why it fails. There are no compilation errors, but once the program actually starts to run, it "thinks" a little bit then crashes. Any ideas?

Relevant code:

void algoritmoDeTorres(int numDiscos, pila &origen, pila &aux, pila &meta)
{
    GotoXY(0,0);
    if(numDiscos==1)
    {
        int item;
        item = origen.pop(); //crashes in this function
        lista laux;
        laux.insertaInicio(item);
        meta.push(item);
        return;
    }
    algoritmoDeTorres(numDiscos - 1, origen, aux, meta);
    origen.imprimePila();
    cout << endl;
    aux.imprimePila();
    cout << endl;
    meta.imprimePila();
    algoritmoDeTorres(numDiscos -1, aux, meta, origen);
}


class pila
{
    private:
        lista lisst;
    public:
        int pop()
        {
            int tam;
            tam = lisst.regresaItem();
            lisst.borraInicio();
            return tam;
        }        
    };

class lista
{
private:
    nodo *cabeza;
public:
    lista()
    {
        cabeza = NULL;
    }
    void borraInicio()
    {
        nodo * aux;
        aux = cabeza->next; 
        delete cabeza;
        cabeza = aux;
    }
    int regresaItem()
    {
        return cabeza->item; //crashes here specifically
    }
};

class nodo
{
public:
    int item;
    nodo* next;

    nodo(int a,nodo * siguiente)
    {
        item = a;
        next = siguiente;
    }
};

int main()
{
    pila ORIGEN,AUX,META;
    ORIGEN.push(3);
    ORIGEN.push(2);
    ORIGEN.push(1);

    algoritmoDeTorres(ORIGEN.Size(),ORIGEN,AUX,META);

    ORIGEN.destruirPila();
    AUX.destruirPila();
    META.destruirPila();
    return 0;
}

PS: Sorry for the Spanglish, my class is in Spanish but many of the ideas are still presented in English, hence the funky language.

Important translations should they be necessary:

aloritmoDeTorres - Towers Algorithm

Discos - Disks

pila - Stack

Origen - Origin

Meta - Destination/Goal

InsertaInicio - Insert(at)Beginning

Imprime - Print

Regresa - Return

Borra - Erase/Delete

Nodo - Node

Cabeza - Head

Siguiente - Next

  • 2
    Read https://ericlippert.com/2014/03/05/how-to-debug-small-programs/ – Passer By Mar 12 '18 at 06:29
  • 1
    Thank you, my bad for the inappropriate post, wasn't aware, I am now. – Jorge Zazueta Mar 12 '18 at 06:35
  • 2
    This code, *exactly* as presented, can't possibly compile, much less run to a point of failure. The number of member functions missing, the order of the functions and class, etc. – WhozCraig Mar 12 '18 at 07:06
  • 1
    @WhozCraig Following with the Debugging Watches I found where the code fails (Segmentation Fault), and it happens in the provided code, I didn't include the rest as it isn't relevant to the failure (I don't think). The pop function actually works when applied directly in the main, so I think the error happens when passing from class to class in the provided code (I'm just not sure why). – Jorge Zazueta Mar 12 '18 at 07:10
  • 1
    `aux->item = ...` look at that `aux` variable. `aux` points to *nothing* determinate. In fact, it's declared on the line above, and was never set to point to *anything*. You're *fortunate* it crashed, as at least you can see where the problem is. And fyi, this problem repeats itself in at least one other place. This is a sign you need to review how pointers work. – WhozCraig Mar 12 '18 at 07:14
  • 1
    @WhozCraig Thanks for the catch! I did change that variable to an int, however, and the crash continues to happen in the same place. – Jorge Zazueta Mar 12 '18 at 07:23
  • Welcome to StackOverflow. Please read and follow the posting guidelines in the help documentation. [Minimal, complete, verifiable example](http://stackoverflow.com/help/mcve) applies here. We cannot effectively help you until you post your MCVE code and accurately describe the problem. We should be able to paste your posted code into a text file and reproduce the problem you described. – Prune Mar 12 '18 at 15:54

1 Answers1

0

Mostly was just rearranging the classes, and filling in the blanks:

#include <iostream>

using namespace std;

class nodo
{
public:
    int item;
    nodo* next;

    nodo(int a,nodo * siguiente)
    {
        item = a;
        next = siguiente;
    }
};

class lista
{
private:
    nodo* cabeza;
    nodo* p;
public:
    lista()
    {
        cabeza = NULL;
        p = NULL;
    }
    void insertaInicio(int n)
    {
        //create a new node
        cabeza = new nodo(n, p);
        //set next pointer to head
        p = cabeza;

    }
    void borraInicio()
    {
        nodo * aux;
        aux = cabeza->next; 
        delete cabeza;
        cabeza = aux;
    }
    int regresaItem()
    {
        return cabeza->item;
    }
};

class pila
{
private:
    //sz (with getters and setters) used to keep track of the size of the stack
    //(incremented in push, decremented in pop)
    int sz;
    lista lisst;
public:
    pila()
    {
        sz = 0;
    }
    int size()
    {
        return sz;
    }
    void setsize(int size)
    {
        sz = size;
    }
    void push(int n)
    {
        lisst.insertaInicio(n);
        sz++;

    }
    int pop()
    {
        int tam;
        tam = lisst.regresaItem();
        lisst.borraInicio();
        sz--;
        return tam;
    }
    void imprimePila()
    {
        //a lengthy way to print the stack
        //was abit unusual as the pop function
        //was removing elements from the stack,
        //when all that was necessary was to print them.
        //this was a workaround that just copied the stack,
        //printed (and removed) elements from original stack,
        //but set the empty to stack to the copy,
        //to leave it unchanged.
        //feel free to find a better way for this
        lista copy;
        cout << '[';
        int lsize = this->size();
        int arr[3] = {};
        int ind = 2;
        while (this->size() != 0){
            int item = this->pop();
            arr[ind] = item;
            cout<< item;
            cout << ',';
            ind--;
        }
        for (int i = ind; i < 3; i++){
            copy.insertaInicio(arr[i]);
        }
        cout <<  ']';
        lisst = copy;
        this->setsize(lsize);
    }
};
//the stacks are instantiated globally so they can be printed at every
//recursive step from algoritmoDeTorres()
pila ORIGEN, AUX, META;

void algoritmoDeTorres(int numDiscos, pila &origen, pila &aux, pila &meta)
{
    //if statement used to rearrange discs ONLY if there are any (if numdiscos is more than 0)
    if (numDiscos > 0){
        //move (numDiscos - 1) discs from origen to aux, so they are out of the way
        algoritmoDeTorres(numDiscos - 1, origen, meta, aux);

        //move the exposed disc from origen to meta
        int item;
        item = origen.pop();
        //lista laux;
        //laux.insertaInicio(item);
        //commented this list out since the item
        //is added to the list of stack its being added to in the push call
        meta.push(item);

        ORIGEN.imprimePila();
        AUX.imprimePila();
        META.imprimePila();
        cout << endl;

        //now move the (numDiscos - 1) discs that we left on aux onto meta
        algoritmoDeTorres(numDiscos - 1, aux, origen, meta);
    }
}

int main()
{
    ORIGEN.push(3);
    ORIGEN.push(2);
    ORIGEN.push(1);

    ORIGEN.imprimePila();
    AUX.imprimePila();
    META.imprimePila();
    cout << endl;

    algoritmoDeTorres(ORIGEN.size(), ORIGEN, AUX, META);
    //objects not allocated with 'new' don't need to be explicitly destroyed
    //ORIGEN.destruirPila();
    //AUX.destruirPila();
    //META.destruirPila();

    return 0;
}
jackw11111
  • 1,457
  • 1
  • 17
  • 34
  • 1
    Thank you! I actually got it working today and, among a few of the things you changed, the main thing that was making the whole code blow up was that I didn't have a constructor setting a new list to NULL. This gave me a lot of grief when I tried to animate the process, but it works now :) thanks again! – Jorge Zazueta Mar 15 '18 at 01:36