WW #4

[Egor Suvorov]

Корректность 1/10 есть.

По стилю:

1. Можно не класть строковой литерал в массив и выводить, а сразу вывести: printf("%s\n", "Hello World");
2. Вообще незачем использовать %s: printf("Hello World\n");
3. В mergesort.c незачем включать <stddef.h> для size_t у параметров mergesort: объявление уже гарантированно есть в mergesort.h => там должен быть включён <stddef.h>

[Egor Suvorov]

Очень хорошо! foreach с пары — это модно :)

Корректность 6.5/7:

1. В mergesort.h оставьте только те заголовки, которые нужны конкретно заголовку. Там требуется только size_t, который лежит в <stddef.h>.
2. В задании просят реализовать копирование памяти руками. memcpy — это хорошо (а memmove — ещё лучше), но конкретно тут напишите, пожалуйста, свою функцию вроде my_memcpy (кажется, вам даже не требуется, чтобы она работала с пересекающимися диапазонами памяти).

По стилю 2/3:

1. Ключ -c традиционно в CFLAGS не включают, а пишут в каждой строчке компиляции отдельно. Это нужно, чтобы можно было просто переиспользовать флаги, если мы хотим и скомпилировать, и слинковать.
2. В mergesort.h следует объявить только my_mergesort, merge используется лишь внутри mergesort.c и ему незачем торчать наружу.
3. Ставьте пробелы перед открывающей фигурной скобкой: while (1) {, а не while (1).
4. В merge можно сделать там один цикл (мне кажется, так будет нагляднее): сделайте внешний цикл по k, а внутри напишите внутри if дизъюнкцию двух условий: либо правая половина кончилась, либо левый элемент меньше.
5. В mergesort незачем писать циклы для копирования массива элементов. У вас уже есть/должна появиться функция для копирования подряд идущих байтов.
6. Объявляйте переменные в самом вложенном месте, где они в первый раз нужны. Например, i в mergesort нужна только в циклах, а вторую
7. Предпочитайте int x = 10 конструкции int x; x = 10. Например, middle в mergesort.

[Egor Suvorov]

Корректность 6.5/7:

1. В mergesort.h оставьте только те функции, которые экспортируются из mergesort.c наружу, т.е. только my_mergesort. Остальные две (my_memcpy, merge) за пределами mergersort.c не используются ("детали реализации") => в mergesort.h не должны возникать.
2. В mergesort.h достаточно только <stddef.h>, в котором лежит size_t. <stdlib.h> нужен для malloc только в реализации, т.е. только в mergesort.c, но не во всех файлах, которые хотят вызывать my_mergesort().
3. Выводятся лишние пробелы на экран (тут foreach, видимо, не особо поможет, лучше честный цикл каждый раз написать, кода больше не станет; ну или заменить foreach на print_elements и пусть сам пробелы расставляет).

Стиль 1.5/3:

1. my_mergesort: пробел между if и return (запустите clang-format, он вам всё это пофиксит).
2. Мне кажется, было лучше, когда была отдельная функция merge: тогда в ней было меньше переменных и нельзя было случайно заиспользовать array вместо left/right.
3. Инициализируйте переменные сразу: size_t middle = elements / 2
4. Объявляйте переменные в самом вложенном месте: например, i.
5. Лучше не цикл while, а цикл for по k, вы заранее знаете количество итераций.
6. Лучше не j/k, а какие-то имена, связанные с именами массивов.
7. Лучше не if (x != NULL), а просто if (x).
8. Можно упростить алгооритм merge: у вас там первое и третье условие совпадают, можно их объединить.
9. Много пустых строк в конце mergesort.c, файл должен завершаться ровно одним переводом строки.
10. printf("%s", "foo"); лучше заменить на printf("foo");

Стало меньше баллов в основном из-за огромной тучи переменных, условий и строк в my_mergesort. Просто выделить merge — уже станет сильно лучше (например, формулы вроде elements - middle заменятся на right_len, что лучше выражает намерение программиста).

[Egor Suvorov]

Корректность 6.5/7:

1. Выводите перевод строки в конце строк (всех).
2. my_mergesort иногда не возвращает -1, когда должен.

Стиль 2/3:

1. #include<stdio.h> — не хватает пробела. Странные переносы строк в сигнатуре merge. Натравите автоформаттер и забудьте про эти проблемы.
2. Объявлять my_memcpy в начале mergesort.c не надо, он потом сразу определяется.
3. foreach уже не foreach, а печатает все элементы через пробел. На это даже имя параметра намекает. Надо переименовать.
4. Арифметика с void* — нестандартное расширение компилятора GCC (предупреждает об этом, если включить флаг -pedantic), лучше сначала кастовать в char*, а потом делать арифметику.
5. my_mergesort можно сильно упростить:
   1. Выделяйте не два дополнительных массива, а один.
   2. Копируейте элементы не в цикле, а одним вызовом my_memcpy.
   3. Уберите неиспользуемую переменную i.
6. merge можно сильно упростить:
   1. Объявляйте переменную array_index в самом цикле, а не заранее.
   2. Объедините условия для 1/4 или 2/3 веток if, останется один if вместо трёх с двумя принициально разными ветками.
   3. Объявите внутри цикла вспомогательные переменные и запишите в них адрес левого/правого элемента, чтобы не было тонны арифметики.

[Egor Suvorov]

По корректности 7/7 (без бонуса):

1. Из-за бонуса иногда возникают утечки памяти.

По стилю 2.5/3:

1. Не хватает пробелов в mergesort.c в одном месте между скобочкой и командой. Натравите автоформатирование.
2. Добавьте в конце main ветку else и assert(!"Unknown type to sort"); в ней, чтобы при передаче неверного аргумента ошибка не заглушалась, а возникала громко.
3. printf("%s", "foo"); лучше заменить на printf("foo");
4. В memcpy промежуточные переменные не нужны.
5. Можно склеить случи в merge попарно. Тогда сокртится количество кода.

[Egor Suvorov]

Забыл замечание по стилю:

6. Пожалуйста, не используйте арифметику с void*. Это расширение GCC. Кастуйте к char* (можно проверить, добавив опцию -pedantic).

[Egor Suvorov]

Корректность и бонус почти полностью, по ним пока 7.5:

1. Не следует делать printf("%s\n", "foo\n");

По стилю осталось (сейчас 2.5/3):

1. Склеить случаи в merge.

[Egor Suvorov]

Кажется, так и осталось.

Захотите дорешать — переоткрывайте тикет.