I have two questions about my code:
- Are there any performance improvements in this implementation or generally (
register int
?) that I could make? - What could I improve in my coding style?
void mergesortArray(int data[], int amount){
if(amount == 1) return;
//Precomputing values
int sizeint = sizeof(int);
int amountLeft = amount / 2;
int amountRight = (amount % 2 == 0) ? amountLeft : (amountLeft + 1);
//Splitting the array in right and left
int *left = calloc(amountLeft, sizeint);
int *right = calloc(amountRight, sizeint);
if(left == NULL || right == NULL) return;
//Copying the splitted content
memcpy(left, data, amountLeft * sizeint);
memcpy(right, data + amountLeft, amountRight * sizeint);
//Recursive sorting the splitted arrays
mergesortArray(left,amountLeft);
mergesortArray(right,amountRight);
//Merging the numbers
int *pos1 = &left[0];
int *pos2 = &right[0];
int i = 0;
for(i = 0; i < amount; i++) {
if(*pos1 <= *pos2) {
data[i] = *pos1;
if (pos1 == &right[amountRight - 1])
break;
if(pos1 == &left[amountLeft - 1])
pos1 = &right[amountRight - 1];
else
pos1++;
}
else {
data[i] = *pos2;
if(pos2 == &right[amountRight - 1])
pos2 = &left[amountLeft - 1];
else
pos2++;
}
}
}
register
modifier is that the address of the variable cannot be acquired. I.E. a total waste of effort. \$\endgroup\$calloc()
twice, but there is no calls tofree()
so this algorithm will leak memory like a sieve. \$\endgroup\$free()
for each of the pointers from the two local calls tocalloc()
just before returning from the recursion level \$\endgroup\$calloc()
andfree()
by sorting/merging the sub arrays in place rather than copying them and sorting the copies \$\endgroup\$