-1

Here is the sample code to create dynamic array in C. I was searching for simple dynamic array, could n't get find the simple one. Implemented my own version.Any suggestions are welcome.

#include <stdio.h>
#include <stdlib.h>

const int MAX_LENGTH_INC=5;

int main(void)
{

    int *ptr,i;
    int maximum_array_size = 1;
    int index=0;
    ptr = malloc(MAX_LENGTH_INC * sizeof(int));

    printf("Address of previously allocated memory: ");
    for(i = 0; i < 8; i++) // incrementing upto maximum of size 8
    {
       //printf("\n Pointer is:%u\t",ptr + i);
       //printf("\nindex is: %d",index);
       ptr[index] = i*2; 
       //printf("\nValue :%d\t",ptr[index]);

       index++;
       if(index == maximum_array_size)
       {
            //printf("\n Array reached its limit");
            ptr= realloc(ptr,sizeof(int)*MAX_LENGTH_INC);
            maximum_array_size = maximum_array_size + MAX_LENGTH_INC;
       }
    }
    for(i=0;i<index;i++)
        printf("\n Array Value is %d ",ptr[i]);
    free(ptr);

  return 0;

}

Habi Sheriff
  • 179
  • 1
  • 1
  • 7

1 Answers1

-1

Ok, you got a few problems here, let me dig through them step by step.

#include <stdio.h>
#include <stdlib.h>

const int MAX_LENGTH_INC=1;

While there is a const keyword in C, I would suggest not using it to define constants like this. Instead, just use a #define like so:

#define MAX_LENGTH_INC 1

However, const int MAX_LENGTH_INC = 1; is not wrong, so you may keep it if you like it.

int main(void)
{

Correct. Alternative: int main(int argc, char * argv[]) {. This way, callers of your program can pass command line arguments. In his program, obviously you don't need it, so void is fine.

    int *ptr,i;
    int maximum_array_size = 1;

So, keep in mind: This is your current size of the array. You actually wanted to start out with MAX_LENGTH_INC, I guess, so better write:

    int maximum_array_size = MAX_LENGTH_INC;

This way, if you increase MAX_LENGTH_INC, you also increase the size of your first allocation, not only of subsequent ones.

    int index=0;
    ptr = (int*) malloc(MAX_LENGTH_INC * sizeof(int));

A comment already noted: Don't cast return value of malloc. Also: You should use variable maximum_array_size here so:

    ptr = malloc(maximum_array_size * sizeof(int));

using sizeof(int) is pretty common style in C, just to note it here: sizeof is not a function but an operator. So sizeof int would be valid, too. /edit: see comment

    printf("Address of previously allocated memory: ");
    for(i = 0; i < 8; ++i)
    {
        //printf("\n Pointer is:%u\t",ptr + i);
        //printf("\nindex is: %d",index);
        ptr[index] = i*2; 
        //printf("\nValue :%d\t",ptr[index]);

        index++;
        if(index == maximum_array_size)
        {
             //printf("\n Array reached its limit");

And here comes your logic problem:

             ptr=(int *)realloc(ptr,sizeof(int)*MAX_LENGTH_INC);
             maximum_array_size = maximum_array_size + MAX_LENGTH_INC;

You always realloc the array to hold MAX_LENGTH_INC elements. You remember then, that it should have grown, but you don't actually grow it. So, first, swap the two lines and then like above in the malloc call, use the variable in the realloc call.

             maximum_array_size = maximum_array_size + MAX_LENGTH_INC;
             ptr = realloc(ptr, maximum_array_size * sizeof(int));

The addition maximum_array_size = maximum_array_size + MAX_LENGTH_INC; can be written as maximum_array_size += MAX_LENGTH_INC;. This actually helps reading the code, because now it's immediately clear, the value MAX_LENGTH_INC will be added to maximum_array_size.

And then you should decide, whether you want to write sizeof(int) * num or num * sizeof(int). I believe, num * sizeof(int) is more logical, because you have 20 ints and not int 20s ;)

Now the other suggestion from a comment was: Incrementing by 1 each iteration would be really slow for long loops. So better double the buffer size each iteration:

             maximum_array_size *= 2;
             ++maximum_array_size; // not exactly needed, I like it this way

Now, you may waste a bit of buffer, but you will at most waste 50%. Of course, you should add a new variable: num_array_elements, because maximum_array_size doesn't count the number of elements any more.

        }
    }
    for(i=0;i<sizeof(ptr);i++)
        printf("\n Array Value is %d ",ptr[i]);

Again a comment already told it: sizeof(ptr) is definitively wrong here. The reason is: ptr is a pointer. And the size of the pointer is always the same (i.e. 8 bytes), regardless of the size of the memory block it points to. You would want to use the new variable num_array_elements here.

Then, even for one liner statements like here, you should add curly brackets. The time comes, you add another line and forget to add the curly brackets then, yielding really hard to find bugs. So:

    for (i = 0; i < num_array_elements; i++) {
        printf("\n Array Value is %d ", ptr[i]);
    }

Also, as you can see, I added some spaces. Spaces make it easier to read your code. Control flow keywords like for and if typically are written with a following space, functions calls without. The language doesn't care, but it is widely adopted style.

    free(ptr);

    return 0;
}

Rest seems fine. So, yet to do for you: Actually add and implement the variable num_array_elements and maybe rename maximum_array_size to either array_size or maximum_array_elements. Also, writing max instead of maximum is widely adopted style as well, like writing num instead of number.

Bodo Thiesen
  • 2,476
  • 18
  • 32
  • 1
    `sizeof 2` works fine, but `sizeof int` fails to compile. You can only omit parentheses for expressions, not types. –  Nov 23 '16 at 11:43
  • Anyway, now I know, why people just stick to writing parentheses all the time. – Bodo Thiesen Nov 23 '16 at 11:55
  • 1
    There's also another reason for the parentheses: `sizeof 2 * 3 + 4` is a bit more difficult to read than `sizeof(2) * 3 + 4` or the equivalent `4 + 3 * sizeof(2)`. Now imagine all of those numbers are expressions themselves. For example, I'd write `arr->cap * 2 * sizeof(*arr->data)` in the growth implementation for my own reusable dynamic array types. –  Nov 23 '16 at 12:12
  • 1
    @ChronoKitsune It's perhaps easier explained that to compute the size of a type, the argument to `sizeof` is a cast expression to that type. Both `2` and `(int)` are expressions, but only the latter is a cast, and of course casts are written with parentheses. I also strongly suggest having a space between `sizeof` and its argument, since *it's not a function*. – unwind Nov 23 '16 at 12:30
  • @unwind Very true, though a cast expression is typically used to convert data to a different type, so it's strange to think of an operation as having a size outside of computer science courses... You're also correct that there should be a space between `sizeof` and its argument. That's actually a by-product of the way I think about allocation. To me, the size of each item being allocated is the visual equivalent of background noise: necessary but requiring less attention. I'm allocating N items, and that's the important part. Perhaps this is wrong of me, but it hasn't failed me yet! :-) –  Nov 23 '16 at 13:01
  • @ChronoKitsune Well yes, the size of the cast is not what is being computed, the cast is used to name a type in an expression. – unwind Nov 23 '16 at 13:04