Tenacity
Tenacity

Reputation: 9

Singly linked list returning an error even though I have already checked it

When using a singly-linked list, I've been suggested to use "if x != nullptr" but I don't understand why this works. In my example code, the first and second print statements work, but as soon as I add the third one, there's a nullptr error. This is despite already checking the node the line before it, and successfully running it.

ListNode* addTwoNumbers(ListNode* l1) { // l1 has 3 nodes containing a single digit
    std::cout << l1->val;
    l1 = l1->next;
    if (l1 != nullptr) {
        std::cout << l1->val;
    }
    std::cout << l1->val;
    return 0;
}

If you comment out the third print statement, it prints out the first two nodes, but if you add the third, it has an error. I have already checked if (l1 != nullptr) and successfully ran the second print statement so it should be safe to assume l1->val is also valid as well, yet it isn't.

Upvotes: -1

Views: 96

Answers (1)

tbxfreeware
tbxfreeware

Reputation: 2186

For production work, you should be using one of the linked-list classes from the Standard Library. Headers <list> (for doubly linked lists) and <forward_list> (for singly linked lists) should be preferred over any sort of "roll-your-own" implementation.

If this is for homework, or possibly some sort of coding challenge, you may not have that option. Note, however, that passing around naked ListNode pointers, and treating them as lists, is the wrong way to do things. If you have to roll your own, you should model your linked list classes after those in the Standard Library. That means coding a proper List or ForwardList class that manages the ListNodes that make up a linked list. The ListNode struct itself, should be nested within the parent List class, and you should write iterators, so that you do not have to work with raw pointers.

It's a pretty big job.

Once again, however, you may not have the option to do things "the right way." LeetCode, for instance, uses naked ListNode pointers such as the ones found in the OP.

Stream insertion operator<<

The OP includes code to output a linked list. That code fails, because it dereferences pointers before checking to see whether they equal nullptr.

Here is an alternative formulation that uses operator<<.

I have added static functions clear, push_front, and pop_front to struct ListNode. This simplifies the construction and tear-down of linked lists. Function clear acts as a sort-of destructor. Calling it at the end of function main prevents memory from leaking.

Nothing is done, however, to implement the "Rule of Three." That may be okay for certain homework assignments, or LeetCode challenges, but that is a big no-no for production code.

// main.cpp
#include <iostream>

struct ListNode
{
    int val{};
    ListNode* next{ nullptr };
    ListNode(
        int const val = 0,
        ListNode* const next = nullptr)
        : val{ val }
        , next{ next }
    {}
    static void clear(ListNode*& head) noexcept {
        while (head)
            pop_front(head);
    }
    static void pop_front(ListNode*& head) noexcept
    {
        if (head) {
            auto p{ head };
            head = head->next;
            p->next = nullptr;
            delete p;
        }
    }
    static void push_front(ListNode*& head, int const n) {
        head = new ListNode(n, head);
    }
    friend std::ostream& operator<< (std::ostream& ost, ListNode* const head)
    {
        ost.put('[');
        if (auto node{ head }; node)
            for (;;) {
                ost << node->val;
                node = node->next;
                if (!node) break;
                ost.put(',');
            }
        ost.put(']');
        return ost;
    }
};
int main()
{
    ListNode* head{ nullptr };
    ListNode::push_front(head, 1);
    ListNode::push_front(head, 2);
    ListNode::push_front(head, 3);
    std::cout << "list = " << head << "\n\n";
    ListNode::clear(head);  // avoid memory leaks.
    std::cout 
        << "after `ListNode::clear(head);`  "
        << "list = " << head << "\n\n";
}
// end file: main.cpp

Output:

list = [3,2,1]

after `ListNode::clear(head);`  list = []

Upvotes: 1

Related Questions