Counting the longest increasing subsequence in an array of integers This is my finished function to find the length of the longest consecutive subsequence of an input array of integers. It's for an assessment so things like the numerical method used and the format of input are out of my hands (int LIS(int* seq, int* temp_seq, int seq_size)).  For the temp_seq parameter, the caller will pass an empty array of size seq_size.

As far as I can tell works perfectly, however, our lecturer has barely commented on how to write code that doesn't just do the job but is 'good code' (aka a large amount of the marking criteria). 

Is there anything that you can see which could be improved here?

In the past I've lost marks for code that did the job, but didn't follow 'good practice' standards. I've tried to make this code as nice as I can, with a clear logical structure, good commenting etc but I always thought my code was good then because when I tested it it worked. I'm just looking for advice on the form of the code.

int LIS(int* seq, int* temp_seq, int seq_size)
{
    /*sets up variables and initial values*/
    int i, proceed_flag;
    int j = 0, count = -1;

    /*this'll be used to ensure each input element is only used once - 1 means not used*/
    int use_flag [seq_size];
    for (i = 0; i&lt;seq_size; i++)
    {
        use_flag[i] = 1;
    }

    /*while the temp array isn't full*/
    while(j&lt;seq_size)
    {
        i = 0;
        proceed_flag = 0;

        /*drops the next unused number into the temp array*/
        while(proceed_flag == 0)
        {
            if (use_flag[i] == 1)
            {
                temp_seq[j+1] = seq[i];
                use_flag[i] = 0;
//              printf("temp_seq = %d\n", temp_seq[j+1]); 
                j++;

                proceed_flag = 1;
            }

            i++;
        }

        /*then drops all smaller numbers from the input array into the temp array*/
        for(i = 0; i&lt;seq_size; i++)
        {
            if(temp_seq[j] &gt; seq[i] &amp;&amp; use_flag[i])
            {
                temp_seq[j+1] = seq[i];
                use_flag[i] = 0;
//              printf("temp_seq = %d\n", temp_seq[j+1]); 
                j++;
            }
        }

        /*this counts the number of repeats - the initialisation value of -1 is just to calibrate the system
        it takes an extra loop around and gives a wrong result otherwise*/
        count++;
    }

return(count);
}
Here is one comment about the readability of your code.

/* 1 means not used */
int used_flag[seq_size];


I allowed myself to change the "use" to "used", because it is more grammatically correct.

Now first of all, if "1 means not used", then you should name this array not_used_flag.

Alternatively, you can keep calling it used_flag, but let 0 denote the case of "not used".

If you choose the second option, then you can initialize the entire array upon declaration:

int used_flag[seq_size] = {0};


Please note that it doesn't yield better performance because it's a non-static local array.

Nevertheless, it does clean up your code a bit...



Another point is implied in a comment at the bottom of your code:

/* the initialization value of -1 is just to calibrate the system.
   it takes an extra loop around and gives a wrong result otherwise. */


It sounds like an ugly workaround for "an extra loop" that should not take place.

So I think that you definitely need to solve it properly instead of working around it.



Last issue (though possibly related to the previous one):

while (j &lt; seq_size)
{
    ...
    temp_seq[j+1] = ...;
}


By entering the loop with j == seq_size-1, you'll be writing beyond the boundaries of temp_seq.

Your code might be working correctly for some input, but beware that it does look suspiciously unsafe.