A few fairly obvious points:
Prefer nullptr
over NULL
Unless you really need to support older compilers, you're generally better off using nullptr
in new code. If you do use nullptr
, you can still support older compilers if really necessary--it's basically just a class that provides conversions for pointer to T and pointer to member.
###Prefer member initializer lists over assignment in a constructor
Prefer member initializer lists over assignment in a constructor
The most obvious example here would be to replace your node
constructor:
Node(T val, Node *nxt=NULL) {
value = val;
next = nxt;
}
...with one using a member initializer list:
Node(T value, Node *next = nullptr) :
value(value),
next(next)
{}
Note that yes, in this case, although the parameter being used to initialize it have the same name, but the compiler can keep track of which is which anyway.
###Rule of three/five/zero
Rule of three/five/zero
Note that in virtually every case, if you provide a destructor and a copy ctor, you also want a copy assignment operator. If you want the better efficiency you can get from move construction/move assignment available in C++11 and later, you'll probably want to add overloads of those that accept rvalue references. I consider it open to debate whether the rule of zero really applies well in this case--I've written a linked list that used smart pointers, but I was never entirely happy with it. It's clumsy but workable in the case of a singly linked list. For a doubly linked list, it's...basically unmanageable.
###Side Note
Side Note
Contrary to your statement, if you don't include a copy ctor, you probably won't get correct results with this sort of class. In particular, the compiler will generate code that does a shallow copy, so only the head pointer in the LinkedList object will be copied. This means if you copy a linked list, you'll end up with both pointing to a single head node. In itself that's not necessarily a problem, but when either of those is destroyed, all the nodes in the list will be destroyed, so in essence you've just destroyed both linked lists, not just the one you intended to. Attempting to use or destroy the one that wasn't supposed to have been destroyed yet will give undefined behavior.
A few fairly obvious points:
Prefer nullptr
over NULL
Unless you really need to support older compilers, you're generally better off using nullptr
in new code. If you do use nullptr
, you can still support older compilers if really necessary--it's basically just a class that provides conversions for pointer to T and pointer to member.
###Prefer member initializer lists over assignment in a constructor
The most obvious example here would be to replace your node
constructor:
Node(T val, Node *nxt=NULL) {
value = val;
next = nxt;
}
...with one using a member initializer list:
Node(T value, Node *next = nullptr) :
value(value),
next(next)
{}
Note that yes, in this case, although the parameter being used to initialize it have the same name, but the compiler can keep track of which is which anyway.
###Rule of three/five/zero
Note that in virtually every case, if you provide a destructor and a copy ctor, you also want a copy assignment operator. If you want the better efficiency you can get from move construction/move assignment available in C++11 and later, you'll probably want to add overloads of those that accept rvalue references. I consider it open to debate whether the rule of zero really applies well in this case--I've written a linked list that used smart pointers, but I was never entirely happy with it. It's clumsy but workable in the case of a singly linked list. For a doubly linked list, it's...basically unmanageable.
###Side Note
Contrary to your statement, if you don't include a copy ctor, you probably won't get correct results with this sort of class. In particular, the compiler will generate code that does a shallow copy, so only the head pointer in the LinkedList object will be copied. This means if you copy a linked list, you'll end up with both pointing to a single head node. In itself that's not necessarily a problem, but when either of those is destroyed, all the nodes in the list will be destroyed, so in essence you've just destroyed both linked lists, not just the one you intended to. Attempting to use or destroy the one that wasn't supposed to have been destroyed yet will give undefined behavior.
A few fairly obvious points:
Prefer nullptr
over NULL
Unless you really need to support older compilers, you're generally better off using nullptr
in new code. If you do use nullptr
, you can still support older compilers if really necessary--it's basically just a class that provides conversions for pointer to T and pointer to member.
Prefer member initializer lists over assignment in a constructor
The most obvious example here would be to replace your node
constructor:
Node(T val, Node *nxt=NULL) {
value = val;
next = nxt;
}
...with one using a member initializer list:
Node(T value, Node *next = nullptr) :
value(value),
next(next)
{}
Note that yes, in this case, although the parameter being used to initialize it have the same name, but the compiler can keep track of which is which anyway.
Rule of three/five/zero
Note that in virtually every case, if you provide a destructor and a copy ctor, you also want a copy assignment operator. If you want the better efficiency you can get from move construction/move assignment available in C++11 and later, you'll probably want to add overloads of those that accept rvalue references. I consider it open to debate whether the rule of zero really applies well in this case--I've written a linked list that used smart pointers, but I was never entirely happy with it. It's clumsy but workable in the case of a singly linked list. For a doubly linked list, it's...basically unmanageable.
Side Note
Contrary to your statement, if you don't include a copy ctor, you probably won't get correct results with this sort of class. In particular, the compiler will generate code that does a shallow copy, so only the head pointer in the LinkedList object will be copied. This means if you copy a linked list, you'll end up with both pointing to a single head node. In itself that's not necessarily a problem, but when either of those is destroyed, all the nodes in the list will be destroyed, so in essence you've just destroyed both linked lists, not just the one you intended to. Attempting to use or destroy the one that wasn't supposed to have been destroyed yet will give undefined behavior.
A few fairly obvious points:
Prefer nullptr
over NULL
Unless you really need to support older compilers, you're generally better off using nullptr
in new code. If you do use nullptr
, you can still support older compilers if really necessary--it's basically just a class that provides conversions for pointer to T and pointer to member.
###Prefer member initializer lists over assignment in a constructor
The most obvious example here would be to replace your node
constructor:
Node(T val, Node *nxt=NULL) {
value = val;
next = nxt;
}
...with one using a member initializer list:
Node(T value, Node *next = nullptr) :
value(value),
next(next)
{}
Note that yes, in this case, although the parameter being used to initialize it have the same name, but the compiler can keep track of which is which anyway.
###Rule of three/five/zero
Note that in virtually every case, if you provide a destructor and a copy ctor, you also want a copy assignment operator. If you want the better efficiency you can get from move construction/move assignment available in C++11 and later, you'll probably want to add overloads of those that accept rvalue references. I consider it open to debate whether the rule of zero really applies well in this case--I've written a linked list that used smart pointers, but I was never entirely happy with it. It's clumsy but workable in the case of a singly linked list. For a doubly linked list, it's...basically unmanageable.
###Side Note
Contrary to your statement, if you don't include a copy ctor, you probably won't get correct results with this sort of class. In particular, the compiler will generate code that does a shallow copy, so only the head pointer in the LinkedList object will be copied. This means if you copy a linked list, you'll end up with both pointing to a single head node. In itself that's not necessarily a problem, but when either of those is destroyed, all the nodes in the list will be destroyed, so in essence you've just destroyed both linked lists, not just the one you intended to. Attempting to use or destroy the one that wasn't supposed to have been destroyed yet will give undefined behavior.