0

I can't figure out why my print functions won't work. It seems that when i create objects of the list they do not point at each other as expected. Somehow when those objects are created they will point to nullptr in both directions.


#include <iostream>
#include <string>
#include <stdexcept>

template <typename T> 
class doubly_list {
private: 
    doubly_list* next_node;
    doubly_list* prev_node;
    T value = T();
public : 
    doubly_list(const T& nvalue, doubly_list* next = nullptr, doubly_list* prev = nullptr)
        : value(nvalue), next_node(next), prev_node(prev) {
    }

    doubly_list* getnext_node() const { return next_node; }
    doubly_list* getprev_node() const { return prev_node; }
    T getlist_value() const { return value; }
    
    doubly_list* move_to_the_next_node() const { return next_node; }
    doubly_list* move_to_the_prev_node() const { return prev_node; }

    doubly_list* insert(doubly_list* next);                       // adding next before prev 
    doubly_list* insert(doubly_list* prev, doubly_list* next);    // adding next before prev 

    void print_the_list(doubly_list<T>* head) const;              //print the list
};

//adding next before prev 
template<typename T>
doubly_list<T>* doubly_list<T>::insert(doubly_list* next) {
    if (this == nullptr) return next; 
    if (next == nullptr) return nullptr;

    next->next_node = this;

    if (this->prev_node) this->prev_node->next_node = next;

    this->prev_node = next->prev_node;
    this->prev_node = next;

    return next;
}
//next before prev
template<typename T>
doubly_list<T>* doubly_list<T>::insert(doubly_list* prev ,doubly_list* next) {
    if (prev == nullptr) return next; 
    if (next == nullptr) return nullptr;

    doubly_list<T>* current_node = next;

    current_node->next_node = prev;
    prev->prev_node = current_node;

    current_node->prev_node = nullptr;


    if (prev->prev_node != nullptr) {
        prev->prev_node->next_node = current_node;

        prev->prev_node = next->prev_node;
    }
        
    return next;
}
//printing the list 
template<typename T> 
void doubly_list<T>::print_the_list(doubly_list<T>* head) const {
    //checking if the list is valid
    if (!head) throw std::invalid_argument("No members found in the list");
    //actual output
    if (head->next_node == nullptr) {
        std::cout << "{" << std::endl;
        while (head) {
            std::cout << head->value;
            head = head->prev_node;
        }
        std::cout << "}" << std::endl; 
    }
    else if (head->prev_node == nullptr) {
        std::cout << "{"; 
        while (head) {
            std::cout << head->value;
            head = head->next_node;
        }
        std::cout << "}" << std::endl; 
    }
}
int main () { 

     auto head = new doubly_list<std::string>("1");
     head->insert(head ,new doubly_list<std::string>("2"));
     head->insert(head ,new doubly_list<std::string>("3"));
     head->insert(head ,new doubly_list<std::string>("4"));

     head->print_the_list(head);
        
     return 0;

}

I am learning c++ with Bjarne Stroustrup's Principles and Practice book and i wanted to implement this using templates but i was following book's guidence .

I thought that it must be some kind of a logical problem , so i've written those functions couple of times but it was pointless .

2
  • 1
    I get why you want to do this as a training exercise, but for real code you probably want to just use std::list - or, in most situations, actually just std::vector since it will usually perform much better than a linked list in real life (unless of course you need the guarantees about not moving elements that a list gives you). Commented May 18, 2024 at 18:23
  • Why auto head = new doubly_list<std::string>("1"); rather than just auto head = doubly_list<std::string>("1"); - No need for dynamic memory allocation (you also forgot to delete - another option would have been std::make_unique). Commented May 18, 2024 at 18:24

1 Answer 1

1

After your insert() calls, none of the nodes are pointing where they should be.

The head's prev_node and next_node members are both nullptr, which is why your print doesn't work.

And in every other node, its prev_node member is still nullptr, and its next_node member is pointing at itself.

In this Demo, I added code to print out each node's members, and this is what I see:

0x557eba303eb0 value=1 prev=0 next=0
0x557eba303ef0 value=2 prev=0 next=0x557eba303ef0
0x557eba303f30 value=3 prev=0 next=0x557eba303f30
0x557eba303f70 value=4 prev=0 next=0x557eba303f70

It should have looked like this instead:

0x557eba303eb0 value=1 prev=0 next=0x557eba303ef0
0x557eba303ef0 value=2 prev=0x557eba303eb0 next=0x557eba303f30
0x557eba303f30 value=3 prev=0x557eba303ef0 next=0x557eba303f70
0x557eba303f70 value=4 prev=0x557eba303f30 next=0

So, you need to review your insert logic in a debugger and fix it, as it is quite broken.

Had you called your 1-param insert() instead of your 2-param insert(), then you can get a viable list - albeit in reverse order since that is the way you implemented its logic:

auto head = new doubly_list<std::string>("1");
head = head->insert(new doubly_list<std::string>("2"));
head = head->insert(new doubly_list<std::string>("3"));
head = head->insert(new doubly_list<std::string>("4"));

head->print_the_list(head); // prints: {4321}

Demo

That being said, in general this is not the best way to design a linked list. Nodes should not manage each other. Create a separate LinkedList class which holds a head (and preferably tail) member, and implement your insert() and print() methods on that class instead.

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

Comments

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.