CODEWITHSUNDEEP
c
user2390775
user2390775

Reputation: 65

Describe what the following C code implements

doing some exam prep and the following is a past question. Describe what the following piece of C code does and rewrite with good programming practices.

int g (int *y, unsigned size,int z){
    int tmp = y [0];
    int * b = y+size;
    y[0] = z;
    while(1)
        if(*(--b)==z) { 
            y[0]= tmp; return b-y;
       };
    y[0] = tmp;
    if(tmp == z) 
        return 0;
    else 
        return -1;
}

Cant seem to figure out what its doing, mainly because of the while(1) does that not mean the while loop will run for ever or is the if statement cancelling the while (1). I have never seen this been done before. Thank you for your help.

Upvotes: 0

Views: 116

Answers (2)

lurker
lurker

Reputation: 58284

What the function will do is return the index in y of the last element in y[] that equals z (of size size). The default is that it will return 0 since it actually assigns z to y[0] before the while (1) loop. So the while (1) is guaranteed to exit when b == y if it finds no other z in y[].

This is a very confused and poorly written function, by the way, since none of the instructions after the while loop block can ever be reached since the while (1) only exits via return.

Then the error is that if there is no z in the array, it will return 0 implying that there is one at location 0.

Upvotes: 1

Filipe Gonçalves
Filipe Gonçalves

Reputation: 21223

The answer by lurker gives a good explanation of what the function does, but it doesn't address how to rewrite it into a more maintainable form.

Here's my suggestion:

int find_last_occurrence(int y[], unsigned size, int z) {
    int first_pos = y[0];
    int i = size;
    y[0] = z;
    while (y[--i] != z)
      ; /* Intentionally left blank */
    y[0] = first_pos;
    return i;
}

Changing y from int * to int [] in the arguments list conveys the idea that y is an array and not a pointer to an integer. Although it ends up being the same in this specific case, using array notation where an array is expected is generally a good choice.

tmp is a poor choice of a variable name, it says nothing about what it holds. first_pos is a better description. I also removed pointer arithmetic altogether and used indexing: pointer arithmetic is easy to get wrong, and it is utterly unnecessary here. Plus, it makes it possible to return immediately without having to do any further computation.

One can argue that while loops with an empty body can be a bad idea, since we're relying on the loop condition's side effects. I find it easy to read and compact, especially when it is clear that the body is empty and a comment says that it is intentional (however, I am fully expecting disapproving comments on this).

As a final touch, you might want to rename the variables to avoid single-letter variable names.

Upvotes: 2

Related Questions