Thread: Linked List implementation

  1. #1
    Registered User
    Join Date
    Sep 2005
    Posts
    20

    Linked List implementation

    I'm trying to implement a list interface in C. It is a sorted list with items declared as
    Code:
    typedef struct {
    	char* str;
    	int len;
    } Item;
    I think the problem is in this function. I can create the list from a data file containing strings and then print it out using another function. However, when printing the list it prints out the same string twice. I will attach all relevant files.
    Code:
    void FindLocation(List* L, ListNode** Prev, Item item)
    {
    	ListNode* Iterator;
    	Item anItem;
    	
    	Iterator = L->head;
    	*Prev = NULL;
    
    	while( Iterator)
    	{
    		*Prev = Iterator;
    		if(strcmp(item.str, Iterator->data.str) > 0 ) 
    		{
    			fprintf(stdout, "The new item is greater than the iterator with data\n%s\n", Iterator->data.str);
    			break;
    		}
    		Iterator = Iterator->next;
    	}
    }
    The output never contains the debug string in the if block in the above function.

    Here is a sample output:
    Code:
    t3\LinkSort\Debug>LinkSort input.data
    Previous node is null.
    New node has data This is line one.
    .
    Head node has data This is line one.
    .
    Previous node has data THis is line two..
    New node has data THis is line two..
    THis is line two.
    THis is line two.

  2. #2
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,057
    Code:
    temp.str = (char*)malloc(sizeof(line));
    Don't cast malloc. And free it at the end of your program, too.

    You don't close the file fp.

    Code:
    item->len = (int)strlen(item->str);
    I don't think you should cast strlen(), either.
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  3. #3
    Bioport Productions
    Join Date
    Oct 2005
    Posts
    215
    is there any reason why you're taking ListNode** Prev and then setting it to NULL!? Clean up your code a bit, I see alot of things that could be left out. Such as Item anItem; that's not being used. Also, are you sure Iterator isn't NULL, why don't you check that. One thing you need to learn as a programmer is to check EVERYTHING again and again. Try printing each string in the while statement to the console for a debug test.
    Last edited by durban; 10-05-2005 at 09:44 PM.
    -"What we wish, we readily believe, and what we ourselves think, we imagine others think also."
    PHP Code:
    sadf 

  4. #4
    Registered User
    Join Date
    Sep 2005
    Posts
    20
    Quote Originally Posted by dwks
    Code:
    temp.str = (char*)malloc(sizeof(line));
    Don't cast malloc. And free it at the end of your program, too.

    You don't close the file fp.

    Code:
    item->len = (int)strlen(item->str);
    I don't think you should cast strlen(), either.
    Thank you for your advice, even though it may be unrelated to the question.

    The code is in no way complete as it stands but I appreciate your tips.

  5. #5
    Registered User
    Join Date
    Sep 2005
    Posts
    20
    Quote Originally Posted by durban
    is there any reason why you're taking ListNode** Prev and then setting it to NULL!?
    Yes because if the previous element is the head of the list, then I will need to add it to the beginning, and I need a way to signal that to the calling function. Thus in the calling method we have:

    Code:
    if(!Prev) 
    //make the new element the head of the list
    Clean up your code a bit, I see alot of things that could be left out.
    Yes I know. I am not finished with it. Anything that seems out of place was at one time for debugging. Once everything works, they will be removed. Thanks for the advice.

    Also, are you sure Iterator isn't NULL, why don't you check that.
    if Iterator is NULL, while(Iterator) become while(NULL) which is while(0) which means the block doesn't execute.

    Then the function that has set *Prev to NULL will return control to the calling function where if the pointer Prev is NULL then we set the current node we are adding to be the head node of the list.

    One thing you need to learn as a programmer is to check EVERYTHING again and again.
    Wait, so testing our logic and implementation is in the description of a programmer?

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. C++ Linked list program need help !!!
    By dcoll025 in forum C++ Programming
    Replies: 1
    Last Post: 04-20-2009, 10:03 AM
  2. Pleas take a look & give a critique
    By sh3rpa in forum C++ Programming
    Replies: 14
    Last Post: 10-19-2007, 10:01 PM
  3. Reverse function for linked list
    By Brigs76 in forum C++ Programming
    Replies: 1
    Last Post: 10-25-2006, 10:01 AM
  4. How can I traverse a huffman tree
    By carrja99 in forum C++ Programming
    Replies: 3
    Last Post: 04-28-2003, 05:46 PM
  5. singly linked list
    By clarinetster in forum C Programming
    Replies: 2
    Last Post: 08-26-2001, 10:21 PM