views:

70

answers:

5

Hi

I'm writing a function that removes the consecutive items with duplicate data . e.g For example, passing in the list

->a->b->c->c->a->b->b->b->a->null

should result in

->a->b->c->a->b->a->null

The list item definition and function declaration are given below

struct litem { 
char data; 
litem* next; 
}; 

Mo code looks like

int remove_dumplicates(litem *&list)
{
 int count = 0; 
 struct litem * current = NULL;
 current = list;
 struct  litem *deleteNode;
 if (current == NULL ) return;
 while(current->next != NULL)
 {
  if ( current->data == current->next->data) // check for the duplicates 
   {
    count++;
    deleteNode =current->next;
    current>next= current->next->next;
    delete deleteNode;
   }      
  return (count);  
 }
}

Is this a correct way of achieving the desired result ?

+2  A: 

I don't see current being incremented to current->next.

Take as an example a list with all unique elements a -> b -> c and see how your program works.

To fix this you need:

while(current->next != NULL) {
   if ( current->data == current->next->data) {
     // delete duplicates .
   } else {
     current = current -> next;
   }
}// end-while
return (count);
codaddict
Why the `current = current->next;` is inside `else` rather than unconditional?
ArunSaha
@Arun: The duplicates are removed inside a `if` and not a `while` so we need to stay at the current position to see if there are more duplicates.
codaddict
We could also do `while(current->data == current->next->data)..remove duplicate` in that case current increment will be unconditional as we are sure that all the duplicates are deleted.
codaddict
That's right, thanks.
ArunSaha
+2  A: 

You need to add an else inside the while loop to advance to the next node:

if( current-> data == current->next->data ) {
....
} else {
    current = current->next;
}

Also the returns need to be fixed (the first should return 0 and the second should be moved outside the while loop).

Why the `current = current->next;` is inside `else` rather than unconditional?
ArunSaha
To take care of multiple duplicates (a->a->a->b)
That's right, thanks.
ArunSaha
I don't know why this got down voted. A +1 from me.
codaddict
+1  A: 
  1. current should move to current->next on no duplicate match.
  2. Argument passed to the function should be merely *list (ie. a pointer to an element of type struct litem)
  3. delete is a C++ keyword. Use free() instead.

The modified code:

int remove_duplicates(struct litem *list)
{
    int count = 0; 
    struct litem * current = NULL;
    current = list;
    struct  litem *deleteNode;
    if (current == NULL ) return;
    while(current->next != NULL)
    {
        if ( current->data == current->next->data) 
        {
            count++;
            deleteNode = current->next;
                current->next= current->next->next;
                    free(deleteNode);
        }
        else
        {
            current = current->next;
        }
    }
    return (count);  
}
Kedar Soparkar
+1  A: 

Some quick observations:

The return (count) statement might be outside the while loop, otherwise the loop would be terminated prematurely.

A statement like current = current->next; is required inside the while loop. Otherwise, the loop would become an infinite loop.

ArunSaha
A: 

Try this:

int remove_duplicates(litem *&list)
{
   int count = 0; 
   struct litem * current = NULL;
   current = list;
   struct  litem *deleteNode;
   if (current == NULL ) return 0;
   while(current->next != NULL)
   {
     if (current->data == current->next->data) // check for the duplicates 
     {
       count++;
       deleteNode =current->next;
       current->next= current->next->next;
       delete deleteNode;
     }
     else
     {
       current = current->next;
     }
   }
   return (count);
 }
Gabe
Thats correct Gabe , thanks :)
ronan