HW #1

[Артур Гулецкий (huletski)]

Корректность

Реализовано load, crop, save. Функциональность не работает на базовом bmp файле (визуально и бинарно файл отличается от оригинала):

{hw_01}[2056]$ pwd && svn up && svn status
/home/hfx/dvl/cpp19/chaykova.anastasiya/hw_01
Updating '.':
Restored 'hw_01'
At revision 1899.
{hw_01}[2057]$ make
mkdir obj
gcc -c -Wall -Wall -Wextra -Werror -Iinclude  src/main.c -o obj/main.o 
gcc -c -Wall -Wall -Wextra -Werror -Iinclude  src/bmp.c -o obj/bmp.o
gcc obj/main.o obj/bmp.o -o hw_01
{hw_01}[2058]$ ./hw_01 crop-rotate samples/lena_512.bmp samples/out_lena_bmp 0 0 512 512
{hw_01}[2059]$ diff samples/lena_512.bmp samples/out_lena_bmp
Binary files samples/lena_512.bmp and samples/out_lena_bmp differ

Прочие замечания:

• make clean не удаляет файл hw_01;
• отсутствует проверка аргументов программы

Стиль

Замечания напишу самые очевидные, т.к. код будет дописываться.

include/bmp.h

• странный выбор идентификатора в guard'e
• параметры crop имеют тип int. Разве предполагается передача отрицательных значений?
• (опционально) передача указателей на потоки ввода/вывода (FILE*) вместо имен файлов увеличит reusability кода работы с bmp.

src/main.c

9,10: макроопределения не используются;
43-45: от глобальных переменных надо избавиться;
56: s/14/sizeof(bitmapFileHeader), аналогично для прочих констант обозначающих размеры структур/смещения, которые можно вычислить/считать из полей;
56: не проверяется успешность выполнения операции чтения;
64: разве floor не избыточен?
93: используйте memcpy для копирования трех байт подряд. Индексы выглядят подозрительно, хотя бы ...+ j + 2*j];

Навскидку, обновляются не все поля заголовков bmp, отвечающие за размеры. Перепроверьте.

--
Итог: корректность 0, стиль 4 (т.к. будет меняться).

[Артур Гулецкий (huletski)]

Корректность

• при сохранении не обновляются поля заголовоков, хранящие размеры файла и изображения. При повороте прямоугольного изображения эти значения могут меняться -> -4;
• bmp.c:31. Проблема с портируемостью: размер int гарантировано 2+ байта, padding может быть 3 -> на системе с sizeof int === 2 программа будет работать неверно с некоторыми изображениями -> -2;

Стиль

fyi перед комментарием - исправлять можно по желанию.

src/main.c

23: не проверяется удалось ли открыть файл.

include/bmp.h

17: имена, состоящие из заглавных букв, обычно используются для макросов.

src/bmp.c

11: (fyi) в C/C++ локальные переменные обычно определяются рядом с местом непосредственного использования, а не в начале функции;
13: отсутствуют проверки на успешность чтения/записи данных из/в файл;
13: что значит 14? Почему не написать sizeof original_image->header_file;
18: что значит 40?
20: (fyi) BYTES_PER_PIXEL можно вычислить так original_image->header_info.biBitCount / CHAR_BIT (плюс assert, что CHAR_BIT === 8, строго говоря);
21, 40: дублирование кода вычисления выравнивания, нужно вынести в функцию;
23: 54?
39: (fyi) имя запутывает: переменная хранит _количество_ байт, а имя говорит, что сами байты;
40: 100+ символов в строке ухудшают читабельность;
69: (fyi) misleading param name: судя по имени первой переменной, вращать можно только обрезанные изображения. Можно ли вращать изображения, для которых предварительно не была вызвана функция crop?

Баллы: корректность 14, стиль 7.5.

[Артур Гулецкий (huletski)]

Корректность

Не обновляется поле include/bmp.h:26 (размер изображения) при изменении файла.

Стиль

src/bmp.c

8: так как предполагается, что функция используется только внутри bmp.c, лучшее ее сделать static;
13,92: отсутствует проверка успешности fread, fwrite (нужно проверить значение, которое вернула функция);
33: использование fseek позволило бы избавиться от переменной buffer;
54: 100+-символьная строка.

Баллы: корректность 18, стиль 9.