Deleting from singly linked list(only problem in code)

[GrizzlyMatt]

Most of my code seems to be okay, I can insert integers into my list and print it out fine, but something about my logic in my deletion seems to be horribly wrong.

if (choice == 'n'){
            cout << "What integer should I delete?\n";
            cin >> num;
            if (num == 0) break;
                n = myList;
                while (n!=NULL){
                    if (n->data == num){
                        n->next=n->next->next;
                    }
                    n=n->next;
                }
                n->next = myList;
                myList=n;

I'm trying to go through my list and if the number isn't there say so, if it is delete it. I'm not doing so well on this list logic, it's steadily destroying my brain. Any hints? This is the only part of my program that isn't working. Here's my full code, and the program simply crashes when I try to delete, no error or anything.

#include <iostream>
using namespace std;

struct Node {
	int data;
	Node *next;
};

int main() {
	int num;
	char choice;
	Node *n, *myList=NULL;
	do{
		cout << "Should I insert? (y/n/q)\n";
		cin >> choice;
		if (choice == 'y'){
		    cout << "What integer should I insert?\n";
            cin >> num;
		if (num == 0) break;
            n = new Node;
            n->data = num;
            n->next = NULL;
		if (myList != NULL) {
			n->next = myList;
			myList = n;
		} else {
			myList = n;
		}
		}
		if (choice == 'n'){
            cout << "What integer should I delete?\n";
            cin >> num;
            if (num == 0) break;
                n = myList;
                while (n!=NULL){
                    if (n->data == num){
                        n->next=n->next->next;
                    }
                    n=n->next;
                }
                n->next = myList;
                myList=n;
		}
		cout << "List=";
		for (n=myList; n!=NULL; n=n->next) cout << n->data << ", ";
            cout << endl;
	}while (choice!='q');
	return 0;
}

I'd like to learn how to properly use linked lists on my own, so if you explain what I'm doing wrong and what I need to change as if I'm a small child I'll really appreciate it.

I looked on google for hours.

[coder777]

yes, your delete logic is wrong. what you need is the predecessor (as another variable). if this predecessor exists (!= NULL) you will set the predecessors next to your currently found n->next. That's it.

line 41/42 are wrong as well: Only when there's no predecessor you set myList = myList->next;

and don't forget to delete the removed item!

[GrizzlyMatt]

So setting the predecessor's next to my currently found n->next will move it forward through the list? is my n->next=n->next->next logic wrong as well? I've been messing with the code and so far I can't even get it to not crash.(the delete part). I'm sorry, I keep reading over tutorials and I've asked a few friends for help understanding, and I can't seem to grasp what I'm doing wrong.

[coder777]

So setting the predecessor's next to my currently found n->next will move it forward through the list?

I'm not sure what you mean. It unlinks n.

You need to delete n and break the while loop. If you don't want to break the while loop (because you want to find more than one number) you need to set n to the predecessor.

is my n->next=n->next->next logic wrong as well?

This will cause a crash if n->next is NULL. Further more you do not unlink the found number but the next

Test if that works:

                Node *p = NULL;
                n = myList;
                while (n!=NULL){
                    if (n->data == num){
                        if(p!=NULL)
                            p->next=n->next;
                        else
                            myList=myList->next;
                        delete n;
                        break;
                    }
                    p=n;
                    n=n->next;
                }

[GrizzlyMatt]

That worked like a charm. Here is my really bad unoptimized finished code in case anyone was wondering/looks this up in the future.

#include <iostream>
using namespace std;

struct Node {
	int data;
	Node *next;
};

int main() {
	int num;
	char choice;
    bool foundNumber=true;
	Node *n, *pre,*myList=NULL;
	do{
		cout << "Should I insert? (y/n/q)\n";
		cin >> choice;
		if (choice == 'y'){
		    cout << "What integer should I insert? \n";
            cin >> num;
		if (num == 0) break;
            n = new Node;
            n->data = num;
            n->next = NULL;
		if (myList != NULL) {
			n->next = myList;
			myList = n;
		} else {
			myList = n;
		}
		}
		if (choice == 'n'){
            cout << "What integer should I delete? \n";
            cin >> num;
            if (num == 0) break;
                Node *p = NULL;
                n = myList;
                while (n!=NULL){
                    if (n->data == num){
                    foundNumber=true;
                        if(p!=NULL)
                            p->next=n->next;
                        else
                            myList=myList->next;
                        delete n;
                        break;
                    }
                    else
                        foundNumber=false;
                    p=n;
                    n=n->next;
                }
		}
		if (!foundNumber)
            cout << num << " is not present in the list\n";
        if(choice == 'q')
            return 0;
		cout << "List=";
		for (n=myList; n!=NULL; n=n->next) cout << n->data << ", ";
            cout << endl;
	}while (choice!='q');
}

Thank you very much for your help. I still don't fully understand it so I'm going to give my book another read through. Take it easy and have a good day :).