I don't know where I am wrong, when I debugged the code I found out that the 'new node' address is 'new node' address, basically the new node is referring to itself

void insertend(struct node *parent, int item)
{
    while (parent->addr != NULL)
        parent = parent->addr;

    struct node new_node;
    new_node.a = item;

    parent->addr = &new_node;
    parent->addr->addr = NULL;
}

CodePudding user response：

I’m not sure what the addr refers to, but I would assume it points to the next node in the linked list. In your code, there needs to be another node to use as a temporary node, because the parent node keeps being reassigned, and the previous items are lost. We also need to check that the parent node is not NULL.

void insertend(struct node **parent, int item)
{
    node *new_node = new node;
    new_node->a = item;
    new_node->addr = NULL;

    if(*parent == NULL)
    {
      *parent = new_node;
    }
    else
    {
      node *temp = *parent;
      while (temp->addr != NULL)
          temp = temp->addr;

      temp->addr = new_node;
    }
}

The if statement checks whether the parent node is NULL. If true, then we assign parent node to new_node. If false, then we go to the end of the list and assign the next node in the list to new_node.

CodePudding user response：

As Igor correctly pointed out, your new node gets destroyed when the function finishes. So, in this context you could just allocate memory. You can use the new operator for this. However, you would need to explicitly free the memory, eventually.

void insertend(struct node *parent, int item)
{
    while (parent->addr != NULL)
        parent = parent->addr;

    struct * new_node = new node;

    new_node->a = item;
    parent->addr = new_node;
    parent->addr->addr = NULL;
}

CodePudding user response：

void insertend(struct node *parent, int item)
{
   while (parent->addr != NULL)
       parent = parent->addr;

   struct node new_node;
   new_node.a = item;

   parent->addr = &new_node;
   parent->addr->addr = NULL;
}

The lifetime of new_node is limited to the function. Once that function returns, it is no longer valid.

In order to circumvent this, it is necessary to dynamically allocate memory for new_node. Of course, as already pointed out, this means explicitly deallocating the memory eventually.

Note: as this is C rather than C, we do not need to add struct to the front of the type in use, and NULL is better spelled nullptr.

void insertend(node *parent, int item)
{
    while (parent->addr != nullptr)
        parent = parent->addr;

    node *new_node = new node;
    new_node.a = item;

    parent->addr = new_node;
    parent->addr->addr = nullptr;
}

As C structs are just classes with default public access, it's also worth noting we could implement this as a member function. Something like:

template <typename T>
struct Node {
    T value;
    Node<T> *next;

    void append(T val) {
        Node<T> * temp = this;
        while (temp->next != nullptr) {
            temp = temp->next;
        }

        temp->next = new Node<T>;
        temp->next->value = val;
        temp->next->next = nullptr;
    }
};

int main() {
    auto n = Node<int>();
    n.value = 27;

    n.append(42);
    n.append(34);

    for (Node<int> *t = &n; t != nullptr; t = t->next) {
        std::cout << t->value << std::endl;
    }

    return 0;
}

The next step would be implementing a constructor and destructor.

One more thing to keep in mind is that getting to the end of a list this way is O(n) time complexity. Doing it over and over again is costly. If you have Node::append return a pointer to the new Node, then you can call append on that.

template <typename T>
struct Node {
    T value;
    Node<T> *next;

    Node<T> *append(T val) {
        Node<T> * temp = this;
        while (temp->next != nullptr) {
            temp = temp->next;
        }

        temp->next = new Node<T>;
        temp->next->value = val;
        temp->next->next = nullptr;

        return temp->next;
    }
};

int main() {
    auto n = Node<int>();
    n.value = 27;

    auto n2 = n.append(42);
    n2 = n2->append(34);
    n2 = n2->append(15);

    for (Node<int> *t = &n; t != nullptr; t = t->next) {
        std::cout << t->value << std::endl;
    }

    return 0;
}