Замечания по Shapes

[alexey-malov]

• [x] Тесты должны запускаться во всех конфигурациях и платформах

[alexey-malov]

• [x] В тестах вместо непонято как полученных значений площадей и периметров (длин сторон треугольника) следует использовать вычисляемые выражения

[alexey-malov]

class CRectangle : public ISolidShape
{
public:
    CRectangle(CPoint const& leftTop, double width, double height, std::string const& outlineColor, std::string const& fillColor);
    ~CRectangle() = default;
    double GetArea() const override;
    double GetPerimeter() const override;
    std::string ToString() const override;
    std::string GetOutlineColor() const override;
    std::string GetFillColor() const override;
    CPoint const& GetLeftTop() const;
    CPoint const& GetRightBottom() const;
    double GetWidth() const;
    double GetHeight() const;
private:
    CPoint m_leftTop;
    CPoint m_rightBottom;
    double m_width;
    double m_height;
    std::string m_outlineColor;
    std::string m_fillColor;
};

• [x] Избыточное хранение данных. Добавьте возможность изменения ширины, высоты, а также координат левого верхнего и правого нижнего углов и убедитесь

[alexey-malov]

• [x] В интерфейсе или классе, унаследованном от интерфейса/класса с виртуальным деструктором деструктор остается виртуальным. Виртуальная функциция в базовом классе остается виртуальной во всех его наследниках

[alexey-malov]

• [x] в классах фигур-наследниках ISolidShape дублируется код метода получения цвета заливки. выделить его реализацию в CSolidShape
• [x] Аналогичное замечание для цвета границы. CShape cделать. И вероятно, потребуется использовать виртуальное наследование.
• [ ] Интерфейс ISolidShape не должен быть выпилен

[alexey-malov]

• [x] ToString имеет повторяющуюся реализацию в классах фигур. Использовать паттерн "Шаблонный метод", пример которого можно посмотреть в примере к объемным телам. Для этого, вероятно, потребуется сделать ToString в верхнем классе, который его реализует, финальным

[alexey-malov]

shared_ptr<IShape> GetMaxAreaShape(vector<shared_ptr<IShape>> &shapes)
{
    return *min_element(shapes.begin(), shapes.end(), [&]
    (shared_ptr<IShape> const& shape1, shared_ptr<IShape> const& shape2)
    {
        return shape1->GetArea() > shape2->GetArea();
    });
}

shared_ptr<IShape> GetMinPerimeterShape(vector<shared_ptr<IShape>> &shapes)
{
    return *min_element(shapes.begin(), shapes.end(), [&]
    (shared_ptr<IShape> const& shape1, shared_ptr<IShape> const& shape2)
    {
        return shape1->GetPerimeter() < shape2->GetPerimeter();
    });
}

• [x] привести отступы в порядок, чтобы было видно вложенность

• [x] функции должны принимать вектор по константной ссылке

• [x] UB при пустом векторе

[alexey-malov]

void PrintShapes(vector<shared_ptr<IShape>> &shapes)
{
    for (auto shape : shapes)
    {
        cout << shape->ToString() << "\n";
    }
    cout << "Shape with maximum area: \n"
        << GetMaxAreaShape(shapes)->ToString() << "\n";
    cout << "Shape with minimum perimeter: \n"
        << GetMinPerimeterShape(shapes)->ToString() << "\n";
}

• [x] UB при пустом векторе
• [x] объекты следует передавать по константной ссылке, если функция не модифицирует их состояние примитивные объекты передавать по константной ссылке выгоды нет, лучше по значению

[alexey-malov]

• [x] Функции Add* содержат дублирующийся код. Лучше сделать функции вида Create, а добавление в коллекцию реализовать в одном месте

• [ ] Пусть вы перегрузили оператор >> для shared_ptr. Но странно, что он просто тупо считывает аргументы без самой строки с типом фигуры. Отдельно без функции, которая его вызывает, он бесполезен.

[alexey-malov]

• [x] Выделить отдельную функцию из main, которая бы обрабатывала ввод и считывала фигуры

[AniSkyWorker]

• [x] создавайте фильтры и распределяйте по ним файлы, чтобы не было огромной портянки http://prntscr.com/dl3zjg

[AniSkyWorker]

• [x] перегруженные методы помечайте override, странно, что сейчас солянка, где-то есть, где-то - нет

  virtual void draw(sf::RenderTarget& target, sf::RenderStates states) const;
  float GetArea() const;
  float GetPerimeter() const;

[AniSkyWorker]

• [ ] stdafx.h в h файлах порождает тучу никому ненужных подстановок и подключений, он должен быть cpp, за исключением очень редких ситуаций

[AniSkyWorker]

• [ ] можно не писать

  CShapeController() = default;
  ~CShapeController() = default;

[AniSkyWorker]

• [x] верхняя верблюжья нотация в названиях функций, тоже самое имена файлов

  void handleEvents(CCanvas &canvas);

[AniSkyWorker]

• [x] stl-объекты передаем по константным ссылкам, если можно

  CSolidShape(std::string shapeType, std::string outlineColor, std::string fillColor);

[AniSkyWorker]

• [ ] у интерфейсов не должно быть реализаций и cpp файлов

  
  ISolidShape::ISolidShape(std::string shapeType, std::string outlineColor)
  : CShape(shapeType, outlineColor)
  {

}

- [ ] наследовать интерфейс от конкретного класса - тяжкое преступление, измените архитектуру(избегайте ромбовидного наследования)
```c++
class ISolidShape : public CShape

[AniSkyWorker]

пока что остановлюсь, k = 0.65

[alexey-malov]

• [ ] sfml носить с собой в виде сабмодуля.

[alexey-malov]

• [ ] Программа собирается с проблемами на компе, отличном от вашего.

[alexey-malov]

• [ ] Программа не должна "забывать" нарисованное на холсте Либо рисуйте в текстуру, либо холст должен запомнить набор примитивов, ранее отправленных на него при рисовании фигур и при отрисовке их рисовать.