WW#09

[Daniil Lyubaev]

Вроде все основные функции работают. Добавил один геттер для id, думаю потом подобавлять еще геттеров и сеттеров.
Еще вопрос, правильно ли я разбил длинное условие в Rectangle::is_inside() ?

[Святослав Власов]

Еще вопрос, правильно ли я разбил длинное условие в Rectangle::is_inside() ?

Так допустимо (только скобки лишние), но чаще пишут вот так:

    return this->x <= x &&
           this->y <= y &&
           this->x + width <= x &&
           this->y + height <= y;

Все тесты попадали по одному и тому же ассерту в Scheme::push_back_figure, поэтому за корректность пока 0.
Если ассерт закомментить, то вылазят следующие ошибки:

1. Rectangle::is_inside работает неправильно. Прочитай внимательно условия, х и у -- это координаты центра фигуры, а не его угла.
2. Очень не хватает функции получения фигуры по id. Это убрало бы много одинакового кода из методов Scheme.

[Daniil Lyubaev]

Поправил, теперь работает. Только в функции Scheme::remove_figure() не стал использовать get_figure_by_id(), потому что там номер тоже нужен.

[Святослав Власов]

1. Зачем нужна странная переменная int num в функции remove_figure? Она никакой полезной функции не несет.
2. Чтобы избавиться от копи-пасты в remove_figure и get_figure_by_id, последняя может возвращать не указатель на фигуру, а её индекс в массиве.
3. Очень просятся ассерты в тех местах, где возврат nullptr из get_figure_by_id может привести к крашу

В остальном мне всё нравится. 9/10

[Daniil Lyubaev]

Поправил. Если вместо ассертов вставить if (get_figure_index(id) == -1) return; , то все работает. А можно как-то не делать несколько ассертов в разных местах, а просто сделать один ассерт в функции get_figure_index()? Я пытался сделать, но у меня не получилось.

[Святослав Власов]

Ассерт работает следующим образом: если условие истинно -- не происходит ничего и программа продолжает исполнение, если условие ложное -- программа падает.
У тебя в remove_figure стоит ассерт assert(index == -1), т.е. программа продолжит работать только если фигура не будет найдена, в противном случае упадет.
Это явно не то, что ты хочешь.

Поправь ассерты и всё будет хорошо.

[Daniil Lyubaev]

Ух, я ещё и кривой ассерт написал... А я думал, что программа специально неправильные айдишники сует. Поправил.

Upd.: Насколько плохим решением было бы убрать ассерты из других функций, а в конец get_figure_index() вместо return -1; вставить assert(0);?

[Святослав Власов]

Теперь всё отлично, 10/10

Upd.: Насколько плохим решением было бы убрать ассерты из других функций, а в конец get_figure_index() вместо return -1; вставить assert(0);?

И тот и другой вариант хороший. Мне тот вариант, который у тебя сейчас, нравится больше -- он позволяет с помощью функции get_figure_index проверить, есть ли фигура с таким айдишником в схеме и обработать вариант, если её нет. Прямо сейчас в этом нет необходимости, но это повышает расширяемость кода, об этом тоже стоит думать.

[Daniil Lyubaev]

Да, я как раз подумал о том, что лучше не ломать программу, если можно её не ломать :)