tags:

views:

120

answers:

2

Hey there, this is my first post so please don't be too hard on me.

I wrote an "insert" function to insert an integer into an array of integers. It works, but I don't know if it's the best algorithm.

Here's my code:

int* insert(int *dest, size_t len, unsigned int index, int value)
{
int x = 0, i = 0;
int *stackp = calloc(len+1, sizeof(int));

if(index > (len-1)) return dest;

while(x < len) {
    if(x == index) {
        ++x;
    } else {
        *(stackp+x) = *(dest+i);
        ++x, ++i;
    }

}


*(stackp+index) = value;
free(dest);
dest = stackp;

return dest;

}

Thanks in advance,

Alex.

+5  A: 

There is a major bug in your memory allocation. stackp is an automatic (stack) array, which means its lifetime ends as soon as insert returns. You have to use another allocation method. You can have the caller allocate a new array and pass in both pointers, or you can do it yourself with malloc (don't forget to free).

However, the rest looks all right. That's pretty much the only algorithm for a non in-place insert. You may be able to do it a bit faster with special tricks (e.g. copying two ints at once). memmove and memcopy may use such optimizations on some architectures.

Also, many algorithms would write stackp[index] when the position is found, rather than at the end. But the core algorithm is basically the same.

An alternative would be to do the insert in-place (shifting only elements after the insert position), rather than using a new array. You would often expand with realloc. This would be preferred in many situations as it saves copying time and avoids mallocing a new memory location (which may also fragment the heap).

Finally, an alternative data structure entirely is a linked list. This totally eliminates the need for copying elements, but uses more memory and prevents random access.

Matthew Flaschen
Thanks for the answer, and the opinion! Performance is not a (critical) issue in this case, so I won't try to optimize my brains out.
Xsander
+2  A: 
eakbas
True, I hadn't noticed that! (It's very, very late here. :P) Also, I believe it's sizeof(int) rather than size(int). Thanks!
Xsander
Right, it should be sizeof :)
eakbas
Do not cast the return value of `malloc`!
R..
Xsander