Member Avatar for amal.sultan.5855

In the Following code, Function Delete_item is not working properly. it only works when we use break; statement... if some can do it without using break; stetement then please share and eplain the method...

#include<iostream>
using namespace std;
struct node
{int info;
node *link;
};
class list
{private:
    node *head, *current, *last, *newnode;
public:
    list()
    {head=NULL;
    current=NULL;
    last=NULL;}
    void create_list_f(int);
    void insertAtN(int, int);
    void delete_item(int data);
    void print();
};
void list::create_list_f(int data)
{
newnode=new node;
newnode->info=data;
newnode->link=NULL;
if(head==NULL)
{head=newnode;
last=head;
}
else
{last->link=newnode;
last=last->link;
}
}
void list::insertAtN(int data, int n)
{int i=1;
newnode=new node;
newnode->info=data;
newnode->link=NULL;
current=head;
if(n==1)
{newnode->link=head;
head=newnode;
}
else
{ 
    while((i<n) &&(current)!=NULL)
{
    if(i==n-1)
        {newnode->link=current->link;
    current->link=newnode;}
    else
current=current->link;
i++;}
}
}
void list::print()
{current=head;
    while(current!=NULL)
{cout<<current->info<<"  ";
current=current->link;
}
}
void list::delete_item(int data)
{node *prev,*q;
q=head;
if(q->info==data)
{q=head;
head=head->link;
delete q;
}
else
{ q=head;
    prev=head;
    while(q!=NULL)
{
    if(q->info==data)
        {prev->link=q->link;
    delete q;}
    else
        {prev=q;
    q=q->link;}}
}}

void main()
{list list1;
int data,item, n;
char choice='y';;
while(choice=='y')
{
cout<<"enter Data"<<endl;
cin>>data;
list1.create_list_f(data);
cout<<"press y if you want to enter more data"<<endl;
cin>>choice;
}
cout<<"Enter the position where you want to insert data"<<endl;
cin>>n;
cout<<"Enter Data"<<endl;
cin>>data;
list1.insertAtN(data, n);
list1.print();
cout<<"Entr element to delete"<<endl;
cin>>item;
list1.delete_item(item);
cout<<"List:"<<endl;
list1.print();
system("pause");
}
Edited by amal.sultan.5855
  • 3 Contributors
  • 2 Replies
  • 428 Views
  • 1 Day Discussion Span
  • Latest Post Latest Post by cheryllocascio

Dani AI

Generated

The real problem is use-after-free and missing null checks, not the loop syntax. 's routine dereferences head without checking for an empty list, and inside the loop it deletes q but never advances q (or updates prev) afterward. A break hides the bug by exiting before the next iteration; 's flag/break idea will stop the crash but doesn't fix the pointer logic. is right to ask why avoid break — it’s fine as a quick fix, but a correct loop is better.

Two safe approaches:

  • Delete only the first matching node: handle the head case, then iterate with prev/curr; when found, update links, delete the node, and return.
  • Delete all matches without using break: first strip matching heads, then walk with prev and curr; when curr matches, set prev->link = curr->link, stash curr in a temp, advance curr to prev->link, then delete the temp. That sequence avoids dereferencing freed memory.

A compact, correct "delete all matches" pattern (replace your function body with this logic):

void list::delete_item(int data) {
    if (head == nullptr) return;

    // remove matching heads
    while (head != nullptr && head->info == data) {
        node* tmp = head;
        head = head->link;
        delete tmp;
    }
    if (head == nullptr) return;

    node* prev = head;
    node* curr = head->link;
    while (curr != nullptr) {
        if (curr->info == data) {
            prev->link = curr->link;
            node* tmp = curr;
            curr = curr->link;
            delete tmp;
        } else {
            prev = curr;
            curr = curr->link;
        }
    }
}

Notes: never check q->info before ensuring q is non-null; do not null out a node’s link before updating surrounding links (that can corrupt the chain); use return when you want to stop after a single deletion. For modern C++, prefer std::forward_list (and its remove/remove_if) to avoid manual pointer mistakes.

Recommended Answers

All 2 Replies

Member Avatar for uonsin

I'm assuming you place the break point is right after you remove the item. You can add a flag or boolean that is set to true after the item is remove and have the loop exit condition to be when q == null or the flag set to true.

I personally will leave it with the break if there is nothing else you need to do after the loop ends and have a message to print after the loop ends to indicate that the item to remove does not exits in the list. Also, I would do q->link = null right before delete q, this is more a personal preference than anything else

Btw, do get any errors without the break? The only thing I can see is that after you delete q, the next loop iteration will set q = null, prev = q (prev = null), and then you do q = q->link which should not work as q is not pointing anywhere

Member Avatar for cheryllocascio

I think the above post would work. I am not sure as I haven't tried, I am hoping so. But I still want to know if the programming code works with the break statement, why do you want to execute it without the break statement

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.