Inserting a no. after some other no. in link list

[Parul Bagadia]

Here is the code i hav written for inserting a no., after given no. in a link list;
i guess the logic is ofcourse right.
there is no error in it,
but at the time of display its not displaying the updated value.
[CODE=cpp]#include<stdio. h>
#include<stdlib .h>
struct linklist
{
int value;
struct linklist*next;
}* first;
//Inserting a number after given number
void insertafter(str uct linklist*first)
{
int p_no;
struct linklist*hold;
struct linklist*temp;
hold=(struct linklist*)mallo c(sizeof(struct linklist));
temp=(struct linklist*)mallo c(sizeof(struct linklist));
printf("\nEnter a particular no. after which you want to add a no.\n");
scanf("\n%d",&p _no);

if(first->value==p_no)
{
printf("\nEnter the desired number:\n");
scanf("\n%d",&h old->value);
hold->next=first->next;
first->next=hold;
first=first->next;
}
else
{
temp=first;
while(temp->next!=NULL &&temp->value!=p_no)
{
temp=temp->next;
}
if(temp->value!=p_no)
{
printf("\nThe number after which you want to add another no. does not exist in list.\n ");
}
else
{
printf("\nEnter the desired number:\n");
scanf("\n%d",&h old->value);
hold->next=temp->next;
temp=hold;
}
}
}
//Display of list
void display(struct linklist*first )
{
printf("\nFollo wing are the elements you have added in the list till now\n");
while(first!=NU LL)
{
printf("\n%d\n" ,first->value);
first=first->next;
}
}[/CODE]

[whodgson]

So what does it print out?

[weaknessforcats]

Why is there data entry in a function that inserts into a linked list? That means you can never use this function anywhere else unless you accept the data entry, which also forces you to accept the screen layout.

What you should do is:

1) do the data entry
2) pass the data, a pointer to the linked list and the position to your insertafter function.

Then insertafter:
1) calls a funciton to create the node by passing the data to it. The function returns a pointer to the new node.
2) traverse the list to locate the insertion point
- be sure to save the next address of the previous node or you can't insert
3) set the new node next to the previous node next
4) set the previous node next to the new node

The insertafter() should have less than 10 lines of code in it.

[Parul Bagadia]

Why is there data entry in a function that inserts into a linked list? That means you can never use this function anywhere else unless you accept the data entry, which also forces you to accept the screen layout.

What you should do is:

1) do the data entry
2) pass the data, a pointer to the linked list and the position to your insertafter function.

Then insertafter:
1) calls a funciton to create the node by passing the data to it. The function returns a pointer to the new node.
2) traverse the list to locate the insertion point
- be sure to save the next address of the previous node or you can't insert
3) set the new node next to the previous node next
4) set the previous node next to the new node

The insertafter() should have less than 10 lines of code in it.

That's one way of doing it; but m sure even my way is right!
I have already written a function for insertion; and not for creating a node!
what output i grt by this is my original list; and not the one i have edited.......
as in the new value ebterwed by this function

[Banfa]

That's one way of doing it; but m sure even my way is right!

"Right" is a very moveable feast. What weaknessforcats suggests is certainly one way of writing this program and what you have done is certainly another.

However it is generally considered best practice to keep your user interface and program functionality separate with a defined interface between them. This normal makes it easier to write, test, maintain and change (if required) either the user interface or the functionality as they are not interfering with each other.

The term "best practice" normally means everyone's consensus on the best approach to take to a problem given the several decades of thought and programming successes and failures that have happened.

The structure weaknessforcats proposes very clearly splits the code into different areas of functionality with user interface and program logic separate. On the other hand your implementation has everything munged together in 1 confusing mass.

[Parul Bagadia]

Thanx for that.
I have made changes in my code and its absolutely working fine now...
Here is the one for inserting a no. after given no,
[code=c]
//Inserting a number after given number.
void insertafter(str uct linklist*first)
{
int p_no;
struct linklist*temp;
struct linklist*assign ;
temp=(struct linklist*)mallo c(sizeof(struct linklist));
assign=(struct linklist*)mallo c(sizeof(struct linklist));
printf("\nEnter a particular no. after which you want to add a no.\n");
scanf("\n%d",&p _no);
if(p_no!=NULL && first!=NULL)
{
temp=first;
while(p_no!=tem p->value && temp->next!=NULL)
{
temp=temp->next;
}
if(temp->value==p_no)
{
printf("\nEnter the no. which you actually want to add \n");
scanf("%d",&ass ign->value);
assign->next=temp->next;
temp->next=assign;
printf("\nThe new edited list is as follows:\n");
display(first);
}
else
{
printf("\nThe no.is not in the list.\n");
}
}
else
{
printf("Please, enter the valid input.");
}
}[/code]
But now my another code for inserting a number before the given no. is not giving me the required output; anyone who's able to suggest sth;
here is the code for it; i even debugged it; and in debuggung its doing all yhe things which i expect from it;
[code=c]
//Insert a number before given number.
void insertbefore(st ruct linklist*first)
{
int take;
struct linklist*store;
struct linklist* bno;
store=(struct linklist*)mallo c(sizeof(struct linklist));
bno=(struct linklist*)mallo c(sizeof(struct linklist));
printf("\nEnter the no. before which you want to add a number\n");
scanf("\n %d",&take);
if(take!=NULL && first!=NULL)
{
store=first;
while(take!=sto re->value && store->next!=NULL)
{
store=store->next;
}
if(store->value==take)
{
printf("\nEnter the no. which you want to add\n");
scanf("%d",&bno->value);
bno->next=store;
store=bno;
printf("\nThe new edited list is as follows:\n");
display(first);
}
else
{
printf("\nThe required no. is not in the list.\n");
}
}

}[/code]
Please reply fast!

[Parul Bagadia]

I need to know this a bit early,
thanx in advance.

[Banfa]

I have made changes in my code and its absolutely working fine now...

Not by my definition of "fine", both of your functions insertafter and insertbefore leak memory, at least sizeof(struct linklist) everytime they are called sometimes twice that.

You function insertbefore is not working because in the heart of the function where you are attempting to insert the new list member
[code=c]
if(store->value==take)
{
printf("\nEnter the no. which you want to add\n");
scanf("%d",&bno->value);
bno->next=store;
store=bno;
printf("\nThe new edited list is as follows:\n");
display(first);
}[/code]
You are only altering local data and data malloc'd in the function, at no time do you alter any of the data in the current list so the current list is unaffected.


Finally you have many posts now, please can you start using &#91;code=c]...&#91;/code] tags when you post code, it makes it a lot easier to read your posts.

[weaknessforcats]

And while you work on insert before and insert after, remember that everything is really insertafter or insertbefore, whichever you choose.

[code=cpp]
insertafter(A,B ); //inserts B after A
insertafter(B,A ) //inserts A after B << really an insert before
[/code]

You have too many functions.

[Parul Bagadia]

Not by my definition of "fine", both of your functions insertafter and insertbefore leak memory, at least sizeof(struct linklist) everytime they are called sometimes twice that.

You function insertbefore is not working because in the heart of the function where you are attempting to insert the new list member
[code=c]
if(store->value==take)
{
printf("\nEnter the no. which you want to add\n");
scanf("%d",&bno->value);
bno->next=store;
store=bno;
printf("\nThe new edited list is as follows:\n");
display(first);
}[/code]
You are only altering local data and data malloc'd in the function, at no time do you alter any of the data in the current list so the current list is unaffected.


Finally you have many posts now, please can you start using [code=c]...[/code] tags when you post code, it makes it a lot easier to read your posts.

Then how come my insertafter is working?
I have done all the same things in it; except pointers are altered in other way.

[Parul Bagadia]

And while you work on insert before and insert after, remember that everything is really insertafter or insertbefore, whichever you choose.

[code=cpp]
insertafter(A,B ); //inserts B after A
insertafter(B,A ) //inserts A after B << really an insert before
[/code]

You have too many functions.

Yaeh, that was good idea but we are not supposed to do so!

[Parul Bagadia]

M still left with the problem as it is!

[Banfa]

I have done all the same things in it; except pointers are altered in other way.

That is exactly the point the difference between insertafter and insertbefore is that in insertafter the things pointed to are altered but in insertbefore the pointer itself is altered, take a look at your own code.

[weaknessforcats]

Yaeh, that was good idea but we are not supposed to do so!

Then use a pass-through function:
[code=cpp]
//Insert B before A
void insertbefore(No de* A, Node* B)
{
insertafter(B,A ); //insert A after B is the same as inserting B before A
}
[/code]

[Parul Bagadia]

Thank you so much ppl, now my code is working because of your help.

//Insert a number before given number.
void insertbefore(st ruct linklist*first)
{
int take;
struct linklist*track;
struct linklist*store;
struct linklist* bno;
store=(struct linklist*)mallo c(sizeof(struct linklist));
bno=(struct linklist*)mallo c(sizeof(struct linklist));
printf("\nEnter the no. before which you want to add a number\n");
scanf("\n %d",&take);
if(take!=NULL && first!=NULL)
{
store=first;
while(take!=sto re->value && store->next!=NULL)
{
store=store->next;
}
if(store->value==take)
{
track=first;
while(track->next!=store)
{
track=track->next;
}
printf("\nEnter the no. which you want to add\n");
scanf("%d",&bno->value);
bno->next=store;
track->next=bno;
printf("\nThe new edited list is as follows:\n");
display(first);
}
else
{
printf("\nThe required no. is not in the list.\n");
}
}

}

I thought, may be if somebody still wants to suggest;on how to make it better.
So, above is the working code.