For loop, then semicolon at the end, and then if/else statements?

Question

5.00/5 (1 vote)

See more:

I know it means to do 'nothing' when a semicolon is at the end of a for statement. But what about this:

C++

    for(cur = nodelist, follow = NULL; cur != NULL && cur->value != n; follow = cur, cur = cur->next);

    if (cur == NULL)
    {
        return nodelist;
    }

    if (follow == NULL)
    {
        nodelist = nodelist->next;
    }
    else
    {
        follow ->next = cur->next;
    }
    
    free(cur);
    return nodelist;
}

Im very confused. This code is for deleting nodes from a linked list, everything works fine but i do not understand what is going on in this code. How can it loop and then do nothing? And then it compares with if/else statements? On top of that

C#

free(cur);
return nodelist;

These 2 lines do not belong in any if/else of for loop statement. How is this all working? Every time it loops is it comparing? If it is why is my code crashing when i nest this whole code in braces? I need some advice, thank you!

Posted 30-Mar-15 15:37pm

kujenjishi

Updated 30-Mar-15 16:07pm

George Jonsson

v2

Add a Solution

Comments

Afzaal Ahmad Zeeshan 30-Mar-15 21:56pm

Loops, or if else blocks are not required to must have curly braces "{ }"... But the thing is, they only execute one command after ). So this won't iterate, but other commands (if else) would be executed.

barneyman 31-Mar-15 0:07am

it's subjective, but that 'for' notation would fail a code-review here - i insist that empty clauses are bound with braces and commented as being empty

kujenjishi 31-Mar-15 0:11am

I understand this is bad style, but I am getting all this from a book. Also I understand very very clearly what the code does, its just the for loop that is really aggravating me.

Sergey Alexandrovich Kryukov 31-Mar-15 5:16am

Why would we even look at this code? It is not self-contained: '}' at the end is not balanced, so many things depend on the opening bracket.
—SA

kujenjishi 31-Mar-15 17:18pm

By the way Sergey Alexandrovich, that dangling brace, belongs to the rest of the function, which is just the declaration of follow and cur, I'm sorry that it was in there, it was not my intention.

3 solutions

Solution 3

In addition to solution 1:

While it obviously works, I would consider this kind of for-loop-use bad style. I would write instead:

C++

cur = nodelist;
follow = NULL;
while (cur != NULL && cur->value != n)
{
    follow = cur;
    cur = cur->next;
}

Posted 30-Mar-15 16:41pm

Sascha Lefèvre

Comments

George Jonsson 30-Mar-15 23:18pm

Good addition.

Solution 2

This is a very interesting piece of code.
lets explain it :

C++

//cur is the currently selected node;
//follow is the mother of current node
 for(cur = nodelist, follow = NULL; /*here nodelist is the top. so nodelist has no mother. thus follow is null*/
cur != NULL && cur->value != n; /*Lets look at the condition. cur is null: here if cur is null then no need to continue. another condition is cur->value is not n; which means, the value we are looking for is found. if value==n which also means that cur is not null.*/
follow = cur, cur = cur->next/*final execution part; first follow getting the cur value; and cur value is getting the next node.(point: next node can be null.)*/
);//about semicolon; semicolon is the statement. in c/c++ an empty semicolon is also a statement.
 
    if (cur == NULL) //here look back at for loop. if cur is null which means nothing to continue; so for loop stops; which also mean the value never matched. this can also mean that nodelist is null.
    {
        return nodelist;
    }
 
    if (follow == NULL) //if the follow is null, this means there is only one node in the linked list; this is the assignment part of for loop. and the value is matched in the first node of the lilst; 
    {
        nodelist = nodelist->next; 
    }
    else //if follow is not null which means linked list has more than one node and the match appeared somewhere in the middle of the linked list. 
    {
        follow ->next = cur->next; // here follow(the mother) is holding the next node of current node. which also means current not is removed from the list. but not deleted from memory
    }
    
    free(cur); //deleting from memroy
    return nodelist;
}

this is a delete procedure from a linked list.

Posted 30-Mar-15 16:40pm

Mohibur Rashid

Add a Solution

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

George Jonsson · Accepted Answer · 2015-03-30T16:00:00

You have misunderstood the "do nothing" part a bit.
The statements inside the for-loop are still executed.

Maybe it is easier if you look upon it this way:

C++

for (cur = nodelist , follow = NULL; cur != NULL && cur->value != n; follow = cur , cur = cur->next)
{
    // For each iteration do nothing
}

[Edit] (Corrected the wording for the assignments)
So, as long as the pointer cur is not NULL and the value of cur->value is not equal to n, follow will be assigned the value of cur, cur will be assigned the value of cur->next and the loop continues.

When the loop exits, the variables are checked and actions are taken.

For this part, it is the cleanup and return statement of the method.

C++

free(cur);  // This frees the memory allocated 
return nodelist;

The thing is that you didn't copy the full method, so I don't know what else is going on.
cur, follow and n are defined elsewhere.

[UPDATE] due to comment
I suppose you mean like this

C++

if (follow == NULL)
{
    nodelist = nodelist->next;
}
else
{
    follow->next = cur->next;
    free(cur);
    return nodelist;
}

The first problem with this is that all branches of your code will not have a return statement. This can of course be fixed like this:

C++

if (follow == NULL)
{
    nodelist = nodelist->next;
    return nodelist;
}
else
{
    follow->next = cur->next;
    free(cur);
    return nodelist;
}

This will pass the compilation, but it will result in a memory leak as the variable cur is not cleared. Again you can fix this:

C++

if (follow == NULL)
{
    nodelist = nodelist->next;
    free(cur);
    return nodelist;
}
else
{
    follow->next = cur->next;
    free(cur);
    return nodelist;
}

This will work but you have added the same code twice, which is always a bad idea.
So the good practice is:

C++

if (follow == NULL)
    nodelist = nodelist->next;
else
    follow->next = cur->next;

free(cur);
return nodelist;

So now we are back where we started.

One issue is that you only copied part of the code.
It should be more like:

C++

List* FooBar(List* nodelist, int n)
{
    List* cur = NULL;
    for(cur = nodelist, follow = NULL; cur != NULL && cur->value != n; follow = cur, cur = cur->next) ;
 
    if (cur == NULL)
    {
        return nodelist;
    }
 
    if (follow == NULL)
    {
        nodelist = nodelist->next;
    }
    else
    {
        follow->next = cur->next;
    }
    
    free(cur);
    return nodelist;
}

(List* is a pointer to an imaginary type called List)

You really should read up on for-loops to get a better understanding of the conditions and statements with in the loop declaration.
C Programming Loops[^]

For loop, then semicolon at the end, and then if/else statements?

3 solutions

Solution 1

Solution 3

Solution 2

Add your solution here

Preview 0

For loop, then semicolon at the end, and then if/else statements?

3 solutions

Solution 1

Solution 3

Solution 2

Add your solution here

Preview 0

Existing Members

...or Join us