Opened 5 years ago
Closed 4 years ago
#481 closed ожидается проверка (задача сдана)
HW #1 (BMP)
| Reported by: | Bagryanova Ekaterina | Owned by: | Sokolov Viacheslav |
|---|---|---|---|
| Component: | HW #1 (BMP) | Version: | 3.0 |
| Keywords: | Cc: |
Description
Насколько я поняла, на эту версию вообще не налагается никаких ограничений на то, что должно работать. У меня пока работает (вроде) только такая версия. Оцените, пожалуйста, на сколько адекватнно я вообще это всё храню и нормально ли я начала)
Только не ругайтесь на ассерты, они будут.
Change History (5)
comment:1 Changed 5 years ago by
| Type: | ожидается проверка → ожидаются исправления |
|---|
comment:2 Changed 4 years ago by
| Type: | ожидаются исправления → ожидается проверка |
|---|---|
| Version: | 1.0 → 2.0 |
comment:3 Changed 4 years ago by
| Type: | ожидается проверка → ожидаются исправления |
|---|
Makefile: не хватает зависимости obj/main.o от bmp.h
biSize - это размер структуры (в нашем случае 40)
Сейчас все работает корректно, если нет никаких неожиданных ситуаций, это замечательно.
Тем не менее, непонятно, почему библиотечные функции вроде crop, rotate, save_bmp берут на себе освобождение внешних ресурсов. В данном простом приложении это может быть удобно, но так делать - плохая практика (представте: у Вас закончилось место на жестком диске, и сразу же компьютер перезагружается и все данные за последнюю неделю безвовратно потеряны. Удобно?). Есть важный Принцип единственной ответственности. Функция имеет свою зону ответственности и не должна делать что-то сверх нее. В одном случае программа пишется так:
- сделай А
- теперь сделай Б
- теперь сделай С
В другом случае получается не линейная структура:
- сделай А
- а теперь если в А все получилось таким образом, то Б тоже можно сделать, а иначе в А все прошло вот так и тогда в принципе Б тоже можно сделать
- ....
Лучше по-другому организовать функции:
- load_bmp считывает BMP - великолепно!
- crop отдает другую BMP, вырезанную из первой - отлично! - и больше ничего. Если не получилось - это уже в точке вызова нужно разбираться, что дальше делать: удалить ли первое изображение, или, может быть, удалить что-то другое и попробовать еще раз.
- аналогично rotate только лишь отдаст повернутое изображение, а оригинал не трогает
- save_bmp пытается сохранить изображение на диск (кстати, fwrite может не получиться - место на диске закончилось, это ожидаемая ситуация - ее тоже нужно обработать) и сообщает, удалось или нет
- в main принимаются решения, когда и что удалить и как быть, если что-то пошло не так
В жизни бывает полезно, скажем, поворачивать картинку, а не делать ее копию, но это делается ради производительности (быстрее / меньше потребление памяти), то есть если в этом есть какая-то необходимость.
comment:4 Changed 4 years ago by
| Type: | ожидаются исправления → ожидается проверка |
|---|---|
| Version: | 2.0 → 3.0 |
comment:5 Changed 4 years ago by
| Resolution: | → задача сдана |
|---|---|
| Status: | assigned → closed |
Отличная работа!
В save_bmp в случае неуспеха нужно всегда закрывать файловый дескриптор
Запуск на crop-rotate lena_512.bmp tmp.bmp 0 0 512 512
приводит к terminated by signal SIGSEGV (Address boundary error)
Стоит добавить assert, что sizeof(Header) такой, как Вы ожидаете.
Возможно, стоит именовать
Header -> BmpHeader?
File -> Bmp
Может быть удобно использовать другую сигнатуру void load_bmp -> Bmp* load_bmp(const char* filename)
Идея с Header хорошая.
Также обращаю внимание, что все ошибки в финальной версии должны обрабатываться, некоторые из текущих ассертов с этой точки зрения некорректные (их нужно будет заменить на что-то другое).