Jump to content

  • Log In with Google      Sign In   
  • Create Account

We're offering banner ads on our site from just $5!

1. Details HERE. 2. GDNet+ Subscriptions HERE. 3. Ad upload HERE.


Problem in Pointers


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
3 replies to this topic

#1 meetjatin88   Members   -  Reputation: 104

Like
0Likes
Like

Posted 18 November 2012 - 10:12 PM

[source lang="cpp"]#include <stdio.h>#include <ctype.h>#include <stdlib.h>#include<conio.h>#include<iostream>struct list{ int data; struct list *next;}*L;//function declaration:void create(int data,struct list **head);void print(struct list **head);int main(){ //list *L; int data =10; for(int i=0;i<3;i++) { //struct list *pList = NULL; create(data++,&L); print(&L); getch(); }}void create(int data,list **head){ list *pNew = new list; if (*head==NULL) { (*head)->next=NULL; (*head)->data=data; //(**head).data=info; //pNew->data =data; //pNew->next=*head; } else { pNew =*head; while(pNew!=NULL) { //*head=(**head).next; pNew=pNew->next; } list *temp =new list; temp->data=data; temp->next=NULL; pNew->next =temp; //(**head).next=(**head).next; } //return 0;}void print(list **head){ list *temp1 = new list; temp1 =*head; while( temp1!=NULL ){std::cout<< temp1->data<<" ";// show the data in the linked listtemp1 = temp1->next; // tranfer the address of 'temp1->next' to 'temp1'}}[/source]

Hello Guys,
I am a bit rusty on pointers,i actually want to implement linked list ,but my compiler is giving error.
i am getting run time error at line
(head)->next=NULL;
(head)->data=data;
Would be great if someone can review the short snippet code and point my mistake.
Any other suggestions are always welcome.
Please note that i have also tried using (**head).data=data; but then also i am still getting runtime error .
i am not sure how to proceeed next.

Sponsor:

#2 Bacterius   Crossbones+   -  Reputation: 9262

Like
0Likes
Like

Posted 18 November 2012 - 10:25 PM

The code as presented will not compile, L has not been declared in main(). If your intention was the "list *L" line. then the reason it crashes is because you have not allocated space for the list which L should point to... you need a malloc (and a free) somewhere.

This is really C code with an cout thrown it, btw, not really C++.


Never mind.

Edited by Bacterius, 18 November 2012 - 11:10 PM.

The slowsort algorithm is a perfect illustration of the multiply and surrender paradigm, which is perhaps the single most important paradigm in the development of reluctant algorithms. The basic multiply and surrender strategy consists in replacing the problem at hand by two or more subproblems, each slightly simpler than the original, and continue multiplying subproblems and subsubproblems recursively in this fashion as long as possible. At some point the subproblems will all become so simple that their solution can no longer be postponed, and we will have to surrender. Experience shows that, in most cases, by the time this point is reached the total work will be substantially higher than what could have been wasted by a more direct approach.

 

- Pessimal Algorithms and Simplexity Analysis


#3 Álvaro   Crossbones+   -  Reputation: 13880

Like
3Likes
Like

Posted 18 November 2012 - 11:08 PM

L is declared globally, but in an unusual place (another symptom of old-C-programmer syndrome).

If your compiler is giving you an error, it would be good for you to post it. It might not mean anything to you, but chances are it does mean something to us, and we can help you understand it.

The section where you check if *head is NULL and then you proceed to fill it up with data should look like an obvious mistake to you.

I can only get this type of code right if I draw a bunch of boxes and arrows on a piece of paper, so I can keep track of what's pointing where in each operation.

#4 rip-off   Moderators   -  Reputation: 8668

Like
2Likes
Like

Posted 19 November 2012 - 08:33 AM

Please note that i have also tried using (**head).data=data; but then also i am still getting runtime error .

foo->bar is equivalent to (*foo).bar, so this change cannot affect the behaviour.

Álvaro is correct, the problem is that you are dereferencing a NULL pointer. Even when you fix that, there is another problem. Your create function leaks a node when the list is non-empty.

A simpler way to write it is to split the actions into two steps, the first being to create and populate the new node, and the second to add the node to the list.
void create(int data, list **head)
{
    list *node = new list;
    node->next = NULL;
    node->data = data;
   
    if(*head == NULL)
    {
	    *head = node;
    }
    else
    {
	    node *end = *head;
	    while(end->next != NULL)
	    {
		    end = end->next;
	    }
	    end->next = node;
    }
}
For a singly linked list, if the order of the list is irrelevant, it is faster to add the new element as the front of the list than the end.

You also leak memory in your print function. There is no need to allocate a new node, simply initialise the temporary node with the head:
void print(list **head)
{
    list *temp1 = *head;
    while(temp1 != NULL)
    {
	    std::cout << temp1->data << " ";
	    temp1 = temp1->next;
    }
}

Your print function would be simplified if it accepted a pointer as its parameter, rather than a pointer to a pointer:
void print(list *head)
{
    list *node = head;
    while(node != NULL)
    {
	    std::cout << node->data << " ";
	    node = node->next;
    }
}
There is no need for a pointer to a pointer because you do not need to modify the callee's pointer in this case.

Some of your headers are C++ specific, so let us write the code in a C++ style:
#include <cstdio>
#include <cctype>
#include <cstdlib>
#include <iostream>
#include <conio.h>

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

void create(list *&head, int data)
{
    list *node = new list;
    node->data = data;
    node->next = null;
    if(head == NULL)
    {
	    head = node;
    }
    else
    {
	    node *end = head;
	    while(end->next != NULL)
	    {
		    end = end->next;
	    }
	    end->next = node;
    }
}
void print(list *head)
{
    list *node = head;
    while(node != NULL)
    {
	    std::cout << node->data << " ";
	    node = node->next;
    }
}

int main()
{
    list *L = NULL;
    int data = 10;
    for(int i = 0 ; i < 3 ; i++)
    {
	    create(L, data++);
	    print(L);
	    getch();
    }
}
The changes were:
  • Change the standard headers includes to the C++ style (<cfoo> rather than <foo.h>
  • Drop the "struct" prefix from declarations
  • Drop the declaration of a global as part of the structure declaration*
  • Using a reference rather than a pointer to implement "write back"
Special note on the global, it is unnecessary so as part of the changes I've made it a local variable. In particular that because it is a local variable it must be initialised to NULL, whereas previously as a global it defaulted to being initialised as NULL.

A final note, modifying the caller's variable is more surprising than the simpler approach of using the function's return value. Here is another alternative:
// ...

list *append(list *head, int data)
{
    list *node = new list;
    node->data = data;
    node->next = null;
    if(head == NULL)
    {
	    return node;
    }
    else
    {
	    node *end = head;
	    while(end->next != NULL)
	    {
		    end = end->next;
	    }
	    end->next = node;
	    return head;
    }
}

int main()
{
    list *L = NULL;
    int data = 10;
    for(int i = 0 ; i < 3 ; i++)
    {
	    L = append(L, data++);
	    print(L);
	    getch();
    }
}





Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS