WW #4

[Sokolov Viacheslav]

Проверка контрактов! ​https://wiki.compscicenter.ru/index.php/C%2B%2B_1MIT_осень_1_2019#.D0.A2.D1.80.D0.B5.D0.B1.D0.BE.D0.B2.D0.B0.D0.BD.D0.B8.D1.8F_.D0.BA.D0.BE.D1.80.D1.80.D0.B5.D0.BA.D1.82.D0.BD.D0.BE.D1.81.D1.82.D0.B8.2C_.D0.BF.D1.80.D0.B5.D0.B4.D1.8A.D1.8F.D0.B2.D0.BB.D1.8F.D0.B5.D0.BC.D1.8B.D0.B5_.D0.BA_.D1.80.D0.B0.D0.B1.D0.BE.D1.82.D0.B0.D0.BC

like за реализацию функции merge_sort

merge_sort_recursive: какая мотивация делать left, right, mid int, а не size_t?

я бы рекомендовал придерживаться практики fail-fast: если что-то пошло не так, сразу же останавливаться, а не делать еще какую-то работу (обработка ошибок в merge_sort_recursive). Чтобы не писать if каждый раз (задумка в этом была, насколько я понял), можно использовать short-circuit evaluation для оператора

(​https://en.cppreference.com/w/c/language/operator_logical).

Кроме того, можно будет просто написать return error;

right - left стоит вынести в отдельную переменную.

В merge можно было бы передавать уже сдвинутый на left (или даже mid) указатель, это бы упростило реализацию функции.

Вместо printf("%s\n", "Error: memory allocation failed"); можно просто
printf("Error: memory allocation failed\n");

malloc:

If size is zero, the behavior is implementation defined (null pointer may be returned, or some non-null pointer may be returned that may not be used to access storage, but has to be passed to free).

​https://en.cppreference.com/w/c/memory/malloc

Так что сейчас случай n=0 не работает корректно.

Общая рекомендация: не использовать в качестве счетчика цикла int (зачем? int знаковый, а счетчик не должен принимать отрицательные значения).

[Sokolov Viacheslav]

Что, если кто-то другой позовет merge_sort?
может ли elements быть 0?
может ли element_size быть 0?
что, если element_size * elements переполняется?

больше const. В куче мест можно дописать const, сделайте это, пожалуйста, когда дойдем до c++ это будет совсем важно. В том числе в компараторах теряется константность:
return strcmp(*(CONST char**)a, *(CONST char**)b);
В my_memcpy, например, source не планируется модифицировать. И таких мест много.

Стоит вынести "Error: memory allocation failed" в именованную константу, чтобы не дублировать

эта ветка дублирует первый if

39 else {
40 my_memcpy(current, (char *)array + element_size * (mid + i2), element_size);
41 i2++;
42 }

можно добиться всего одного ветвления

Не принципиально, но можно вместо

 63     if (merge_sort_recursive(array, mid, element_size, comparator) == -1 ||
 64             merge_sort_recursive(array + mid * element_size, elements - mid, element_size, comparator) == -1 ||
 65             merge(array, mid, elements, element_size, comparator) == -1) {
 66 
 67         return -1;
 68     }
 69     return 0;

сделать

return merge_sort_recursive(array, mid, element_size, comparator) 
    || merge_sort_recursive(array + mid * element_size, elements - mid, element_size, comparator)
    || merge(array, mid, elements, element_size, comparator);