HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Single Linked-List Implementation in C++

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
listimplementationsinglelinked

Problem

I haven't done much C++ coding the last few years, so I've been reviewing in preparation for upcoming interviews. I wrote a minimally functional singly linked-list. I'm using my implementation for practicing interview problems and hopefully later to build more complex data structures.

Currently, the code is not templatized since I've been focusing on the basics more than wide usability. I've made the head and tail pointers protected because of how I code up practice problems - each one is a derived class where I implement the required functions and they often require modifying head and/or tail.

Style-wise, I've at least tried to use the Google C++ Style Guide. Please provide feedback on the overall design, improvements, style, and others.

Header:

#ifndef LINKED_LIST_H
#define LINKED_LIST_H
struct Node
{
    int data;
    Node *next = nullptr;
};

class LinkedList
{
    public:
        LinkedList();
        LinkedList(const LinkedList &list);
        ~LinkedList();
        LinkedList & operator=(const LinkedList &other);
        LinkedList & operator+=(const LinkedList &rhs);
        friend LinkedList operator+(const LinkedList &lhs, const LinkedList &rhs);

        bool IsEmpty() const;
        void Append(int value); // add to end
        void Insert(int value); // add to beginning
        void Print() const;

    protected:       
        // Protected so that derived class can modify these directly
        Node *head;
        Node *tail; // allows O(1) append

    private:
        void swap(LinkedList &other);

};

#endif


Implementation:

```
#include
#include "linked_list.h"

LinkedList::LinkedList(): head(nullptr), tail(nullptr)
{
}

LinkedList::LinkedList(const LinkedList &list): LinkedList() // call base constructor
{
Node *curr = list.head;
while(curr != nullptr)
{
this->Append(curr->data);
curr = curr->next;
}
}

LinkedList::~LinkedList()
{
Node *curr = head;
while (curr != nullptr)
{

Solution

If, instead of just writing new, you write new (std::nothrow), then if new fails, it will return a nullptr instead of throwing an exception. Then check if the result was nullptr. Some basic error checking like this isn't going to slow your program down if you're already using linked lists, which are inherently of the devil when it comes to performance.

Also, I don't think an employer would like to see code that might cause bugs. They rather see slower code than code that will cost them development time, I suspect. (I am definitely not an expert on that subject, though.)

I'm also not sure why you keep writing this-> in lots of places, such as this->Append(curr->data);. In case you didn't know, this-> is implicit, so you're only cluttering the code.

At the end of the destructor, you've written head = tail = nullptr;. I'm not sure what the point is, since the very next line, those member functions will be gone as the object is deleted. Just a thought. Perhaps you have some reason for it, I'm just pointing it out as I see it.

Your assignment operators do not check for self-assignment. At the start, check to see if this == &rhs. Even if it still "works", you're making a copy of "yourself", and then swapping yourself with the copy, which is clearly useless.

It would also probably be a good idea to see if the list you're trying to add in operator+= is actually empty, in which case you shouldn't do anything.

Lastly, you should mark all functions that do not throw any exceptions as noexcept, similarly to how you mark some functions as const. That is, unless you're using some older version of C++, for some reason.

I would personally make Struct Node a member structure of LinkedList, but that is a design choice, really, and not of much relevance.

Other than this, it looks pretty neat and good. Good work, I'd say. If I missed anything, perhaps others will care to point those things out.

Context

StackExchange Code Review Q#125837, answer score: 3

Revisions (0)

No revisions yet.