current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 3 down vote favorite
1

The function remove_odd is to remove odd valued nodes (surprise!!) from the linked list head. The code is very long and cryptic but it seems to work.

Please help me reduce the lines of code and are there any cases that I might have missed.

void rem_odd(struct Node** head)
{
    struct Node* curr=*head;
    struct Node* l_even=NULL; //l_even stores the last even number node address
    while(curr!=NULL&&curr->data%2!=0)
    {
        *head=curr->next;
        if(curr->next==NULL)
            return;
        curr=curr->next;
    }
    l_even=curr;
    while(curr!=NULL)
    {
        if(curr->next==NULL)
            return;
        while(curr!=NULL&&curr->next->data%2!=0)
        {

            if (curr->data%2!=0)
            {
                printf("if ");
                if(curr->next==NULL)
                {
                    l_even->next=NULL;
                    return;
                }
                curr=curr->next;
                l_even->next=curr;
                if(curr->next==NULL)
                {
                    l_even->next=NULL;
                    return;
                }
                continue;
            }
            curr=curr->next;
            if(curr->data %2!=0 &&curr->next==NULL)
            {
                l_even->next=NULL;
                return;
            }
        }
        if(curr->next==NULL)
        {
            l_even->next=NULL;
        }
        else
        {
            l_even->next=curr->next;
            l_even=curr->next;
        }
        curr=curr->next;
    }
}
share|improve this question
up vote 1 down vote accepted

It seems you can simplify this by quite a lot. I added explanation of the algorithm inside the code, that way it might be easier to follow:

void rem_odd(struct Node** head)
{
    struct Node* curr = *head;
    struct Node* l_even;

    // iterate until the first even element, or the end
    while (curr != NULL && curr->data % 2 != 0)
    {
        curr = curr->next;
    }

    // the new head is the first even element, or the end
    *head = curr;

    // if no more elements, we're done here
    if (curr == NULL) {
        return;
    }

    // the last even element is the current element, naturally
    l_even = curr;

    // we know the current element is even, and not null, so moving on
    curr = curr->next;

    // iterate until the end
    while (curr != NULL)
    {
        // is the current element even?
        if (curr->data % 2 == 0) {

            // -> yes, it's even, so the last even element should point to it
            l_even->next = curr;

            // ... and this is the new last-even-element now
            l_even = curr;
        }
        curr = curr->next;
    }

    // last even might still point to an odd valued element, so NULL it out
    l_even->next = NULL;
}

Notice that I added spaces around the operators. For example this is extremely hard to read:

while(curr!=NULL&&curr->data%2!=0)
share|improve this answer
    
The NULL'ing of l_even was something new to me,nice trick!. – solinvictus Mar 8 '15 at 15:08

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.