Skip to main content
deleted 1 character in body
Source Link

One thing that I don't like is code such as the following whenever you deal with an empty container:

std::cout << std::endl << std::endl;
std::cout << "\t" << "LL-> Node (" << location << ") Not Found In The List.";
std::cout << std::endl << std::endl;

When you move your code into a class, as @πάντα ῥεῖ said, then you should probably throw an exception (You can make your own class and inherit it from std::exception) instead of couting something. Otherwise, your function is given too many responsibilites, and not all applications for a linked list have a console to output to.

The other thing I don't like is this conditional:

if (head == temp1 && tail == temp1) {
  head = nullptr;
  delete temp1;
} else if (temp1 == head) {
  head = head->next;
  delete temp1;
} else if (temp1 == tail) {
  temp2->next = nullptr;
  tail = temp2;
  delete temp1;
} else {
  temp2->next = temp1->next;
  delete temp1;
}

Notice how there is a delete temp1 at the end of every one of those conditionals. That breaks thatthe D.R.Y principle. Make the delete temp1 unconditional.

One thing that I don't like is code such as the following whenever you deal with an empty container:

std::cout << std::endl << std::endl;
std::cout << "\t" << "LL-> Node (" << location << ") Not Found In The List.";
std::cout << std::endl << std::endl;

When you move your code into a class, as @πάντα ῥεῖ said, then you should probably throw an exception (You can make your own class and inherit it from std::exception) instead of couting something. Otherwise, your function is given too many responsibilites, and not all applications for a linked list have a console to output to.

The other thing I don't like is this conditional:

if (head == temp1 && tail == temp1) {
  head = nullptr;
  delete temp1;
} else if (temp1 == head) {
  head = head->next;
  delete temp1;
} else if (temp1 == tail) {
  temp2->next = nullptr;
  tail = temp2;
  delete temp1;
} else {
  temp2->next = temp1->next;
  delete temp1;
}

Notice how there is a delete temp1 at the end of every one of those conditionals. That breaks that D.R.Y principle. Make the delete temp1 unconditional.

One thing that I don't like is code such as the following whenever you deal with an empty container:

std::cout << std::endl << std::endl;
std::cout << "\t" << "LL-> Node (" << location << ") Not Found In The List.";
std::cout << std::endl << std::endl;

When you move your code into a class, as @πάντα ῥεῖ said, then you should probably throw an exception (You can make your own class and inherit it from std::exception) instead of couting something. Otherwise, your function is given too many responsibilites, and not all applications for a linked list have a console to output to.

The other thing I don't like is this conditional:

if (head == temp1 && tail == temp1) {
  head = nullptr;
  delete temp1;
} else if (temp1 == head) {
  head = head->next;
  delete temp1;
} else if (temp1 == tail) {
  temp2->next = nullptr;
  tail = temp2;
  delete temp1;
} else {
  temp2->next = temp1->next;
  delete temp1;
}

Notice how there is a delete temp1 at the end of every one of those conditionals. That breaks the D.R.Y principle. Make the delete temp1 unconditional.

added 195 characters in body
Source Link

One thing that I don't like is code such as the following whenever you deal with an empty container:

std::cout << std::endl << std::endl;
std::cout << "\t" << "LL-> Node (" << location << ") Not Found In The List.";
std::cout << std::endl << std::endl;

Whenever you find a nullptr. When you move your code into a class, as @πάντα ῥεῖ said, then you should probably throwthrow an exception (You can make your own class and inherit it from std::exception) instead of couting something. Otherwise, your function is given too many responsibilites, and not all applications for a linked list have a console to output to.

The other thing I don't like is this conditional:

if (head == temp1 && tail == temp1) {
  head = nullptr;
  delete temp1;
} else if (temp1 == head) {
  head = head->next;
  delete temp1;
} else if (temp1 == tail) {
  temp2->next = nullptr;
  tail = temp2;
  delete temp1;
} else {
  temp2->next = temp1->next;
  delete temp1;
}

Notice how there is a delete temp1 at the end of every one of those conditionals. That breaks that D.R.Y principle. Make the delete temp1 unconditional.

One thing that I don't like is code such as:

std::cout << std::endl << std::endl;
std::cout << "\t" << "LL-> Node (" << location << ") Not Found In The List.";
std::cout << std::endl << std::endl;

Whenever you find a nullptr. When you move your code into a class, as @πάντα ῥεῖ said, then you should probably throw an exception instead of couting something. Otherwise, your function is given too many responsibilites, and not all applications for a linked list have a console to output to.

The other thing I don't like is this conditional:

if (head == temp1 && tail == temp1) {
  head = nullptr;
  delete temp1;
} else if (temp1 == head) {
  head = head->next;
  delete temp1;
} else if (temp1 == tail) {
  temp2->next = nullptr;
  tail = temp2;
  delete temp1;
} else {
  temp2->next = temp1->next;
  delete temp1;
}

Notice how there is a delete temp1 at the end of every one of those conditionals. That breaks that D.R.Y principle. Make the delete temp1 unconditional.

One thing that I don't like is code such as the following whenever you deal with an empty container:

std::cout << std::endl << std::endl;
std::cout << "\t" << "LL-> Node (" << location << ") Not Found In The List.";
std::cout << std::endl << std::endl;

When you move your code into a class, as @πάντα ῥεῖ said, then you should probably throw an exception (You can make your own class and inherit it from std::exception) instead of couting something. Otherwise, your function is given too many responsibilites, and not all applications for a linked list have a console to output to.

The other thing I don't like is this conditional:

if (head == temp1 && tail == temp1) {
  head = nullptr;
  delete temp1;
} else if (temp1 == head) {
  head = head->next;
  delete temp1;
} else if (temp1 == tail) {
  temp2->next = nullptr;
  tail = temp2;
  delete temp1;
} else {
  temp2->next = temp1->next;
  delete temp1;
}

Notice how there is a delete temp1 at the end of every one of those conditionals. That breaks that D.R.Y principle. Make the delete temp1 unconditional.

Source Link

One thing that I don't like is code such as:

std::cout << std::endl << std::endl;
std::cout << "\t" << "LL-> Node (" << location << ") Not Found In The List.";
std::cout << std::endl << std::endl;

Whenever you find a nullptr. When you move your code into a class, as @πάντα ῥεῖ said, then you should probably throw an exception instead of couting something. Otherwise, your function is given too many responsibilites, and not all applications for a linked list have a console to output to.

The other thing I don't like is this conditional:

if (head == temp1 && tail == temp1) {
  head = nullptr;
  delete temp1;
} else if (temp1 == head) {
  head = head->next;
  delete temp1;
} else if (temp1 == tail) {
  temp2->next = nullptr;
  tail = temp2;
  delete temp1;
} else {
  temp2->next = temp1->next;
  delete temp1;
}

Notice how there is a delete temp1 at the end of every one of those conditionals. That breaks that D.R.Y principle. Make the delete temp1 unconditional.