Skip to content

Latest commit

 

History

History
86 lines (46 loc) · 7.84 KB

common-mistakes.md

File metadata and controls

86 lines (46 loc) · 7.84 KB
Raw

  • bmp_header маленький, всего-то 60 байт. Можно не создавать его в куче, а выделить в стеке.

  • image маленький. Можно не создавать его в куче, а выделить в стеке и возвращать по значению.

  • Эта функция нужна только в одной части программы, которая работает с BMP файлами. Нет смысла делать её доступной извне. Можно определить только в .c файле и пометить как static.

  • padding считается многократно в разных частях программы. Можно сделать для этого отдельную функцию.

  • Не используйте битовые операции для подсчёта padding. Компилятор оптимизирует то, что действительно безопасно оптимизировать.

  • Можно создавать header из struct image, тогда он сразу будет корректным. Хорошо, когда в коде не появляются не до конца сконструированные объекты.

  • Обработки ошибок в fread/fwrite нет.

  • При объявлении структуры как локальной переменной все её поля содержат мусор. Было бы неплохо прямо тут указать значения всех её полей. Или хотя бы инициализировать = {0} .

  • Разумно сделать функцию, которая будет считать адрес пикселя по его координатам.

  • Можно совместить объявление и инициализацию.

  • Сравниваются числа разных типов.

  • Массив вместо switch удобнее для сопоставления строчек вариантам enumа.

  • Сообщения о том, как выполняется программа — в stderr; её результаты — в stdout.

  • Функция — кандидат на static.

  • Как правило именуем функции, работающие со структурами, используя префикс имя_структуры.

  • "Магические числа"

  • Эти функции принадлежат другой части программы, работающей с внутренним представлением картинки, не с bmp файлами.

  • Память под структуры выделять calloc не очень правильно. Лучше malloc + инициализация с помощью ={0}. Причина: "инициализировать нулевыми значениями" != "заполнить нулевыми байтами". Нулевые значения у float/double не обязательно во всх битах имеют нули. Нулевые указатели NULL не обязательно состоят из исключительно нулевых байтов (на некоторых архитектурах могут быть наоборот все биты установлены, т.е. -1).

  • Генерацию заголовка разумно выделить в отдельную функцию.

  • Проверку заголовка на корректность разумно выделить в отдельную функцию.

  • Любой указатель неявно преобразуется из void* в любой другой тип; каст не нужен.

  • Локальные переменные, которые не будут меняться, например, которые создаются для удобства, чтобы меньше писать, полезно пометить const.

  • Переменная объявляется вне цикла, но используется только внутри. Почему бы не совместить инициализацию и объявление?

  • Не стоит возвращать из функции строку об ошибке. Причина: её сложно анализировать. Неочевидно, как в вызывающей функции сделать проверку "а что именно пошло не так в bmp_reader?" и на основании её сделать то или иное действие. Придётся сравнивать строки, что долго. А ещё сообщения об ошибках могут быть на разных языках. Поэтому и делают перечисления чтобы сообщать о результатах выполнения функции.

  • Не надо принимать заголовок BMP файла в аргументах функции, доступной для других частей программы! Это вынуждает вызывающего эту функцию вообще знать про заголовки bmp файлов, а ему это не нужно. Он хочет просто из картинки сделать .bmp файл. Тем более что bmp заголовок генерируется из image.

  • Пользователю трансформаций не хочется вообще знать про заголовки bmp файлов, ему это не нужно. Он хочет просто из картинки сделать .bmp файл. Поэтому всё про заголовок надо скрыть от него. В view-header была написана программа для отображения заголовков bmp файлов, а не программа для поворота картинок.

  • view-header это утилита для просмотра заголовков bmp файлов, а не заготовка для вашей программы, которой bmp заголовки неважны.

  • Игнорируете ремарку про архитектуру, например, объявляя struct image там же, где и функции для работы с bmp файлом. Если мы хотим расширяемости и упрощения работы с частями программы, нужно выделить минимум три структурных части программы: адаптер для bmp -файлов, описание struct image и функций для работы с ней, и трансформации над image. Еслли у вас в рамках одного файла есть определения из более, чем одной части, что-то не так.

  • Память была выделена перед return в строчке ХХХ, теперь перед return с кодом ошибки её надо освободить.

  • Для примера возьмём:

uint8_t value = 0; 

fwrite( &value, padding, 1, file);

Здесь fwrite записывает в файл padding * 1 байт подряд начиная с адреса &value.

  • Такие маленькие функции, использующиеся только в одном файле, стоит помечать static. Тогда они не только могут быть встроены в местах вызова, но компилятор может удалить их определение из файла вообще.

  • Не стоит смешивать в одном проекте camelCase и snake_case.