CODEWITHSUNDEEP
c++linked-list
Tobias Fizzlewig
Tobias Fizzlewig

Reputation: 111

How to fix the inconsistent reading of my linked list?

In a recent project I've been recreating a stack using a linked list, and have been trying to output my way up the linked list, however it refuses to output anything but the head of the stack. While this would not normally vex me, I use a very similar method to test my removal function and it worked as it should. For context into this, each node contains either a char or int variable as well as a next pointer, which the node's default constructor makes NULL. I have accessors and mutators for the And value (an int) and the Or variable (a char).

main.cpp

#include "node.h"
#include "lilis.h"
#include <iostream>
using namespace std;

int main()
{
    lilis obj;
    char a = '%';
    char b = '+';
    char c = '=';
    char d = '-';
    obj.addPush(a);
    obj.addPush(b);
    obj.addPush(c);
    obj.addPush(d);
    obj.display();
    //obj.rePop();

    return 0;
}

lilis.h

#ifndef LILIS_H
#define LILIS_H

class lilis
{
    private:
        node* head;
    public:
        lilis();
        lilis(node* dupe);
        lilis(lilis& dup);
        void addPush(int a);
        void addPush(char b);
        void rePop();
        void display();
};

#endif

lilis.cpp (the commented block of code at the end is what I was trying to get to work, I replaced it so it wouldn't loop infinitely) 

#include "node.h"
#include "lilis.h"
#include "iostream"
using namespace std;

lilis::lilis()
{
    //ctor
}

lilis::lilis(node* dupe)
{
    head=dupe;
}

lilis::lilis(lilis& dup)
{
    //ctor
    head = dup.head;
}

void lilis::addPush(int a)
{
    node* after;
    node* store = head;
    after->setAnd(a);
    after->setNext(head);
    head=after;

}

void lilis::addPush(char b)
{
    node* after;
    node* store = head;
    after->setOr(b);
    after->setNext(head);
    head=after;

}

void lilis::rePop()
{
    node* storage = head;
    cout << head->getOr();
    head = head->getNext();
    cout << head->getAnd();
    delete storage;
}

void lilis::display()
{
    node* after = head;
    cout << after->getOr();
    after = after->getNext();
    cout << after->getOr();
    /*while (after->getNext()!=NULL){
        std::cout << " " <<after->getAnd();
        after = after->getNext();
    }*/
}

Upvotes: 0

Views: 61

Answers (1)

Kiruahxh
Kiruahxh

Reputation: 2035

Your code contains multiple memory instanciation issues. First, the head variable should be initialized in the constructor, because it's a pointer (ex : head = nullptr).

There is the same problem in this function :

void lilis::addPush(int a)
{
    node* after;
    node* store = head;
    after->setAnd(a);
    after->setNext(head);
    head=after;
}

after variable is not initialized, the pointer may contain some random value. You should rewrite it something like this : 

void lilis::addPush(int a)
{
    node* after = new node(); // instanciate a new node, that will be kept in memory after this function returns
    after->setAnd(a);
    after->setNext(head);
    head=after;
}

The display function is realy close. However, there is still one main problem : you must handle the case when your list has no elements (eg head is null) it the display and the rePop functions.

Good luck !

Tip : lilis(node* dupe); is unuseful : node is some internal thing, it should not be exposed in the public interface, delete it or at least make it private...

Upvotes: 1

Related Questions