PR for review#19
Conversation
SPetrenko17
requested review from
leshiy1295
and removed request for
leshiy1295
leshiy1295
left a comment
leshiy1295
left a comment
There was a problem hiding this comment.
Неплохо используется STL
Стоит учесть все замечания, чтобы улучшить архитектуру.
Ну и опять же - здорово, что есть рабочий прототип!
|
|
||
| #include "PageData.h" | ||
|
|
||
| PageData::PageData(std::vector<UserData *> u, PageType t, Organize o) { |
There was a problem hiding this comment.
используйте список инициализации
| #include<vector> | ||
| #include <Views/UserData/UserData.h> | ||
|
|
||
| enum class PageType : uint8_t { |
There was a problem hiding this comment.
зачем int8_t, да ещё и unsigned?
| #include "PageGenerator.h" | ||
|
|
||
| void | ||
| PageGenerator::organizeCells(Organize o, int &rows, int &cells, int count) { |
There was a problem hiding this comment.
не нужно использовать однобуквенные переменные
| rows = count; | ||
| break; | ||
|
|
||
| case Organize::RECT_HORIZONTAL: |
There was a problem hiding this comment.
что с этими вариантами?
| void | ||
| PageGenerator::organizeCells(Organize o, int &rows, int &cells, int count) { | ||
| switch (o) { | ||
| case Organize::ONE_LINE_VERTICAL: |
There was a problem hiding this comment.
стоило вынести в стратегию и реализовать в виде цепочки ответственности/подмешивания policy для конкретного класса
| } | ||
|
|
||
| void CellView::destroy() { | ||
|
|
There was a problem hiding this comment.
что здесь должно происходить?
почему этот метод не вызывается хотя бы в деструкторе?
|
|
||
| void destroy() override; | ||
|
|
||
| std::string toString(int depth) override; |
There was a problem hiding this comment.
вывод и иерархия - вообще-то разные вещи
можно задавать string-методы с помощью примеси (применив паттерн CRTP)
| case BClass::LIST_ITEM: | ||
| return "list-group-item"; | ||
|
|
||
| case BClass::TABLE : |
There was a problem hiding this comment.
кажется, что лучше, если каждый класс эту информацию будет знать про себя сам
иначе этот файл (а он интерфейсный) будет постоянно расширяться...
| std::make_shared<RowView>(*new RowView(0, | ||
| *new TextView( | ||
| "ageView", | ||
| Type::H4, |
There was a problem hiding this comment.
снова магические строки...
| } | ||
|
|
||
|
|
||
| void PersonView::makeTemplate(){ |
There was a problem hiding this comment.
стоило бы шаблон задавать реальным файлом извне, чтобы его мог задавать пользователь без необходимости лезть в исходный код
2 participants
No description provided.