0

First of all I want to create an dynamic LinkedList with Pointers without using STL. It is an exercise to understand how pointers work in C++.

My problem is that my push() method doesn't create a FifoElement like I would expect. When I runstart the program it shows me nothing after I use the pop() method. An example of my main is below.

To push new objects into the List I'm overloading the <<operator and to pop the first element from the list i'm overloading the >>operator

My two classes are Fifo and FifoElement

Fifo:

template <class T>
class Fifo
{
    public:
        Fifo();

        void push(const T&);
        T pop();

        Fifo& operator<<(const T&);
        Fifo& operator>>(T&);


    private:
        FifoElement<T> *top;
};

FifoElement:

template <class T>
class Fifo;

template<class T>
class FifoElement
{
    friend class Fifo<T>;

    public:
        FifoElement();

    private:
        T value;
        FifoElement<T> *next;
};

snippet of code for push method:

...
template <class T>
void Fifo<T>::push(const T& val){
    //creates new Element
    FifoElement<T> *newElem = new FifoElement<T>(); //<= I think my problem is here but I am not sure..

    //inserts new value into Element
    newElem->value = val;

    //If Fifo/List is empty: Element is top...
    if(top == nullptr){
        top = newElem; //...füge am Anfang ein
        return;
    }

    //or:
    //when List has elements go to the end and then insert element at the end
    FifoElement<T> *tmpElem = top;
    while(tmpElem->next != nullptr){
        tmpElem = tmpElem->next;
        cout << tmpElem << endl;
    }
    //set new Element as next of the last element in the list 
    tmpElem->next = newElem;
}

template <class T>
Fifo<T>& Fifo<T>::operator<<(const T& val){
    push(val);
    return *this;
}
...

and the pop() method part:

...
template <class T>
T Fifo<T>::pop(){
    //should read the first element of the list and delete it...
    FifoElement<T> *tmpElem = nullptr;
    FifoElement<T> *returnElem = nullptr;

    //if top is empty it means that the list is empty...
    if(top == nullptr){
        cout << "Liste ist leer!" << endl;
        returnElem = 0;
        return returnElem->value;
    }
    //the new element is the new top
    else if(top->next != nullptr){
        top = top->next;
        returnElem = tmpElem;
        //hier wird Element gelöscht!
        delete tmpElem;
        tmpElem = nullptr;
        //
        return returnElem->value;

    }

    //if only top exists then return it and delete it after that
    else{
        delete top;
        top = nullptr;
        returnElem = tmpElem;
        delete tmpElem;
        tmpElem = nullptr;
        return returnElem->value;
    }
}

template <class T>
Fifo<T>& Fifo<T>::operator>>(T& val){
    pop();
    return *this;
}
...

an example of my main would be like this:

    ...
    int main() {
    Fifo<string> test;
    string ex = "Hey stackoverflow whatsup? :)";
    string ex2 = "can anyone help me?";
    test << ex;
    test << ex2
    test.pop(); //output here should be: "Hey, stackoverflow whatsup? :)"
    test.pop(); //output here should be: "can anyone help me?"
    ...
    return 0;
}

I hope that I didn't forget anything. It would be nice if someone could help me out. I'm sitting on that program since 3 days :(

2
  • Your conclusions are off. Drop them, and debug your pop method, paying close attention to the values of your local variables. Commented Dec 2, 2019 at 18:59
  • 1
    The pop method makes no sense at all. returnElem was never set to top, it's set to tmpElem and both are nullptr at that point. Then you're deleting tmpElem after setting returnElem=tmpElem which will also delete contents of returnElem given all you did was a shallow copy (just setting pointers). Commented Dec 2, 2019 at 19:52

1 Answer 1

2

Your pop method was too complex and contained errors, especially with memory handling. Doing something like

returnElem = tmpElem;
delete tmpElem; 

is wrong because returnElem just became a dangling pointer since you did not create a copy of the FifoElement<T> and both pointers are pointing to the same element. I've rewritten it as

template <class T>
T Fifo<T>::pop(){
    T value;
    if( top ){
        auto const old = top;
        top = top->next;
        value = old->value;  
        delete old;
    } else {
        cerr << "error - queue empty";
    }
    return value;
}

With that change your sample code works. I have not checked it for additional errors. See working example here

Sign up to request clarification or add additional context in comments.

3 Comments

He cannot use STL which I believe includes std::unique_ptr
Yup, now it meets the requirements. The pop method the OP wrote gave me chuckles. It's the weirdest pop I've seen so far.
One little thing, maybe nitpicking, but top = top.next; should be enough, no? Given if next == nullptr it will automatically become top = nullptr.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.