Creating a generic single linked list, follow up

Question

This is a thread I started from here. I organized my code to make it more organized. Everything works well, but I have not made all the changes from the last link (namely namespace since I am exactly sure why or what to do).

Overall I just wanted to show my generic single linked list I made and see if there are any errors or improvements I should make.

Here is my header file:

#ifndef SingleLinkedLists_h
#define SingleLinkedLists_h
#include <iostream>
template <class T>
class SingleLinkedLists {
private:
 struct Node {
 T data;
 Node* next;
 };
 Node* head;
 Node* tail;
public:
 // Constructors
 SingleLinkedLists() : head(nullptr), tail(nullptr) {}
 SingleLinkedLists(SingleLinkedLists const& value);
 ~SingleLinkedLists();
 // Overloaded operators
 SingleLinkedLists& operator=(SingleLinkedLists const& rhs);
 friend std::ostream& operator<<(std::ostream& str, SingleLinkedLists<T>& data) {
 data.display(str);
 return str;
 }
 // Operators in Single Linked List
 void swap(SingleLinkedLists& other) noexcept;
 void createNode(const T& theData);
 void createNode(T&& theData);
 void display(std::ostream& str) const;
 void display() const;
 void insertHead(const T& theData);
 void insertTail(const T& theData);
 void insertPosition(int pos, const T& theData);
 void deleteHead();
 void deleteTail();
 void deletePosition(int pos);
 bool search(const T& x);
};
template <class T>
SingleLinkedLists<T>::SingleLinkedLists(SingleLinkedLists const& value) : head(nullptr), tail(nullptr) {
 for(Node* loop = value->head; loop != nullptr; loop = loop->next) {
 createNode(loop->data);
 }
}
template <class T>
SingleLinkedLists<T>::~SingleLinkedLists() {
 while(head != nullptr)
 deleteHead();
}
template <class T>
SingleLinkedLists<T>& SingleLinkedLists<T>::operator=(SingleLinkedLists const& rhs) {
 SingleLinkedLists copy(rhs);
 swap(copy);
}
template <class T>
void SingleLinkedLists<T>::swap(SingleLinkedLists& other) noexcept {
 using std::swap;
 swap(head, other.head);
 swap(tail, other.tail);
}
template <class T>
void SingleLinkedLists<T>::createNode(const T& theData) {
 Node* newNode = new Node;
 newNode->data = theData;
 newNode->next = nullptr;
 if(head == nullptr) {
 head = newNode;
 tail = newNode;
 newNode = nullptr;
 }
 else {
 tail->next = newNode;
 tail = newNode;
 }
}
template <class T>
void SingleLinkedLists<T>::createNode(T&& theData) {
 Node* newNode = new Node;
 newNode->data = std::move(theData);
 newNode->next = nullptr;
 if(head == nullptr) {
 head = newNode;
 tail = newNode;
 newNode = nullptr;
 }
 else {
 tail->next = newNode;
 tail = newNode;
 }
}
template <class T>
void SingleLinkedLists<T>::display(std::ostream& str) const {
 for(Node* loop = head; loop != nullptr; loop = loop->next) {
 str << loop->data << "\t";
 }
 str << "\n";
}
template <class T>
void SingleLinkedLists<T>::display() const {
 Node* newNode = head;
 while(newNode != nullptr) {
 std::cout << newNode->data << "\t";
 newNode = newNode->next;
 }
}
template <class T>
void SingleLinkedLists<T>::insertHead(const T& theData) {
 Node* newNode = new Node;
 newNode->data = theData;
 newNode->next = head;
 head = newNode;
}
template <class T>
void SingleLinkedLists<T>::insertTail(const T& theData) {
 Node* newNode = new Node;
 newNode->data = theData;
 tail->next = newNode;
 tail = newNode;
}
template <class T>
void SingleLinkedLists<T>::insertPosition(int pos, const T& theData) {
 Node* previous = new Node;
 Node* current = head;
 Node* newNode = new Node;
 for(int i = 1; i < pos; i++) {
 previous = current;
 current = current->next;
 }
 newNode->data = theData;
 previous->next = newNode;
 newNode->next = current;
}
template <class T>
void SingleLinkedLists<T>::deleteHead() {
 Node* old = head;
 head = head->next;
 delete old;
}
template <class T>
void SingleLinkedLists<T>::deleteTail() {
 Node* previous = nullptr;
 Node* current = head;
 while(current->next != nullptr) {
 previous = current;
 current = current->next;
 }
 tail = previous;
 previous->next = nullptr;
 delete current;
}
template <class T>
void SingleLinkedLists<T>::deletePosition(int pos) {
 Node* previous = new Node;
 Node* current = head;
 for(int i = 1; i < pos; i++) {
 previous = current;
 current = current->next;
 }
 previous->next = current->next;
}
template <class T>
bool SingleLinkedLists<T>::search(const T &x) {
 Node* current = head;
 while(current != nullptr) {
 if(current->data == x)
 return true;
 current = current->next;
 }
 return false;
}
#endif /* SingleLinkedLists_h */

Here is the main.cpp file that tests the functions:

#include <iostream>
#include "SingleLinkedLists.h"
int main(int argc, const char * argv[]) {
 SingleLinkedLists<int> obj;
 obj.createNode(2);
 obj.createNode(4);
 obj.createNode(6);
 obj.createNode(8);
 obj.createNode(10);
 std::cout<<"\n--------------------------------------------------\n";
 std::cout<<"---------------Displaying All nodes---------------";
 std::cout<<"\n--------------------------------------------------\n";
 std::cout << obj << std::endl;
 std::cout<<"\n--------------------------------------------------\n";
 std::cout<<"-----------------Inserting At End-----------------";
 std::cout<<"\n--------------------------------------------------\n";
 obj.insertTail(20);
 std::cout << obj << std::endl;
 std::cout<<"\n--------------------------------------------------\n";
 std::cout<<"----------------Inserting At Start----------------";
 std::cout<<"\n--------------------------------------------------\n";
 obj.insertHead(50);
 std::cout << obj << std::endl;
 std::cout<<"\n--------------------------------------------------\n";
 std::cout<<"-------------Inserting At Particular--------------";
 std::cout<<"\n--------------------------------------------------\n";
 obj.insertPosition(5,60);
 std::cout << obj << std::endl;
 std::cout<<"\n--------------------------------------------------\n";
 std::cout<<"----------------Deleting At Start-----------------";
 std::cout<<"\n--------------------------------------------------\n";
 obj.deleteHead();
 std::cout << obj << std::endl;
 std::cout<<"\n--------------------------------------------------\n";
 std::cout<<"----------------Deleting At End-----------------";
 std::cout<<"\n--------------------------------------------------\n";
 obj.deleteTail();
 std::cout << obj << std::endl;
 std::cout<<"\n--------------------------------------------------\n";
 std::cout<<"--------------Deleting At Particular--------------";
 std::cout<<"\n--------------------------------------------------\n";
 obj.deletePosition(4);
 std::cout << obj << std::endl;
 std::cout << std::endl;
 obj.search(8) ? printf("Yes"):printf("No");
 return 0;
}

As an aside I want to thank everyone who has contributed to improving my generic single linked list, I really appreciate the help and expertise that strengthened my understanding of data structures and C++.

• JDługosz: My earlier comments still apply.
• yuri: I organized my code to make it more organized. The perfect description for any refactoring.
• Snorrlaxxx: @yuri Is that sarcasm? I have made changes following the last post, just not all.

Answer 1

I'm not a C++ programmer, take what I say with a grain of salt.

• Don't add articles to names. theData should simply be data. You're not writing a novel, you're writing code.

• Always use braces around while-loops (the same goes for similar constructs, like if). This avoids particularly nasty bugs in the future.

• Single characters (like '\n' and '\t') can be C++ chars, instead of strings:

  str << loop->data << '\t';
  

  ... and:

  str << '\n'
  

• In C++, the '*' / '&' should be part of the type, not of the name. You still missed this in a couple of cases. Here:

  bool SingleLinkedLists<T>::search(const T &x) {
  

  ... and here:

  int main(int argc, const char * argv[]) {
  

  ... and possibly elsewhere too.

• I expect search() to return an index, not a bool. A more logical name might be has() or contains().

• For obj.search(8) ? printf("Yes"):printf("No");, you may be better off just expanding this to use C++ functionality (std::cout).

• Maybe it was intentional here, but don't use std::endl if a newline ('\n') would suffice.

• Indentation is a blessing, not a curse! Use it to distinguish different parts of your code visually. If you really want to limit it, simply use two spaces.

• For all intents and purposes (modern C++) you don't need an explicit return 0; at the end of the main function.

• JDługosz: I disagree with "always use braces". I like Kevlin’s view: instead, ask yourself "why is it more than one statement?" or something like that.
• Daniel: @JDługosz 'why is it more ...' There's many cases where such bodies consist of more than one line, and those do not signify bad design. Do you want to take the risk of running into mysterious bugs as soon as you add another line?
• JDługosz: I’ve never had a problem with that, and I don’t see why it is such a big risk that it’s worth compromising the day-to-day lucidity of reading and working with the file. You want to argue about not using == ever because it might get confused with = some day? I remember that particular issue was all the rage some years back, and some new languages at the time were designed to require a comparison as the top-level expression in the if. But C++, now, specifically supports declare/assign/test all at once.
• JDługosz: I seriously doubt that, in the referenced article from Apple, the programmer meant to add a second line to a block under the if. The first statement is a goto so flow would not return to run a second statement! More likely, a line got duplicated or another if line was deleted or moved at some point.
• Daniel: @JDługosz I see that my argument about the Apple bug was incorrect. I stand by 'better safe than sorry' nevertheless.