Замечания по фигурам

[alexey-malov]

• [x] Не нужно терять интерфейс IShape. В интерфейсе не должно быть данных и методов, имеющих реализацию. Для устранения дублирования кода можно ввести промежуточный абстрактный класс CShape, где реализовать поддержку цвета обводки и, возможно, каркас метода ToString. Если подобное будет сделано и для CSolidShape, то для устранения проблем ромбовидного наследования, которое может появиться, использовать виртуальное наследование. См. пример из лекции

• [x] Данных в интерфейсе также не должно быть

[alexey-malov]

• [x] Определиться с виртуальностью/невиртуальностью методов базового класса

[alexey-malov]

    CreateShape createShape(std::cin);

    while (auto shape = createShape.ExecuteCommand())

• [x] Имя класса должно выражаться существительным (как правило)

• [x] Файлы исходного кода переименовать

• [x] От ShapeCreator-а ожидаешь возможность CreateShape

[alexey-malov]

• [x] C.35: A base class destructor should be either public and virtual, or protected and nonvirtual. Разобраться и уметь объяснить

[alexey-malov]

• [x] Устранить дублирование кода между методами ToString() классами фигур. Один из возможных вариантов - паттерн "Шаблонный метод". Пример с объёмными телами в репозитории oop/lab4

[alexey-malov]

std::shared_ptr<IShape> ShapeCreator::CreateRectangle(std::istream& args) const
{
    CPoint leftTop, rigthBottom;
    std::string outlineColor, fillColor, inputValue;
    try
    {
        args >> inputValue;
        leftTop.x = std::stod(inputValue);
        args >> inputValue;
        leftTop.y = std::stod(inputValue);

        inputValue.clear();

        args >> inputValue;
        rigthBottom.x = std::stod(inputValue);
        args >> inputValue;
        rigthBottom.y = std::stod(inputValue);

        args >> outlineColor;

        args >> fillColor;
    }
    catch (const std::exception& e)
    {
        std::cout << e.what() << std::endl;
        return nullptr;
    }
    return std::make_shared<CRectangle>(leftTop, rigthBottom, outlineColor, fillColor);
}

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

[alexey-malov]

    ShapeCreator inputShape(std::cin);

    while (auto shape = inputShape.ExecuteCommand())
    {
        shapes.push_back(shape);
    }

• [ ] inputShape- неудачное имя

[alexey-malov]

• [x] Добить поиск фигур с максимальной площадью и минимальны периметром

[alexey-malov]

• [x] Настроить автозапуск тестов после сборки во всех конфигурациях

[alexey-malov]

• [x] В ccircle.cpp не хватает подключения stdafx.h

• [x] stdafx.h должен подключаться только в .cpp файлах

[alexey-malov]

• [x] Сократить длину строк в методах ToString

[alexey-malov]

• [x] Обратить внимание на пробелы в конце строк в тестах и в коде

[alexey-malov]

• [x] Тесты разных классов лучше раскидать по разным .cpp файлам

[alexey-malov]

• [x] Выпилить пустые .cpp файлы, не содержащие ничего кроме #include-ов (кроме stdafx.h)

[alexey-malov]

class ISolidShape : public CShape
{
public:
    virtual ~ISolidShape(){};
    virtual std::string GetFillColor() const = 0;

private:
    virtual void AppendProperties(std::ostream& strm) const = 0;
};

• [x] Интерфейсы не должны наследоваться от абстрактных классов. Приватной или защищённой секции в них также не должно быть.

[alexey-malov]

    double m_density;
    std::string m_type;

• [x] Выпилить из CShape

[alexey-malov]

• [x] AppendProperties в дочерних классах сделать приватным, он не предназанчен для вызова извне

• [x] В CShape также он должен оставаться приватным

[alexey-malov]

class CRectangle final : public CSolidShape
{
private:
    CPoint m_leftTop;
    CPoint m_rightBottom;
    double m_width;
    double m_height;
};

• [x] Не храните данные в избытке

[alexey-malov]

• [x] Исправить длинные строки

[alexey-malov]

• [x] В CCircle Должен быть AppendProperties

[alexey-malov]

1>c:\teaching\ips\2018\oop\dorohov\oop\lab4\lab4\csolidshape.h(15): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\ips\2018\oop\dorohov\oop\lab4\lab4\cshape.h(9): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\ips\2018\oop\dorohov\oop\lab4\lab4\csolidshape.h(15): warning C4250: 'CSolidShape': inherits 'CShape::CShape::ToString' via dominance
1>c:\teaching\ips\2018\oop\dorohov\oop\lab4\lab4\cshape.h(10): note: see declaration of 'CShape::ToString'
1>c:\teaching\ips\2018\oop\dorohov\oop\lab4\lab4\ccircle.h(21): warning C4250: 'CCircle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\ips\2018\oop\dorohov\oop\lab4\lab4\cshape.h(9): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\ips\2018\oop\dorohov\oop\lab4\lab4\ccircle.h(21): warning C4250: 'CCircle': inherits 'CShape::CShape::ToString' via dominance
1>c:\teaching\ips\2018\oop\dorohov\oop\lab4\lab4\cshape.h(10): note: see declaration of 'CShape::ToString'

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

[alexey-malov]

• [x] Использовать только один из override,final,virtual в методах

[alexey-malov]

• [ ] Не инклудить из ISolidShape файл CShape

[alexey-malov]

class CShape : public virtual IShape
{
public:
    std::string GetOutlineColor() const override;
    std::string ToString() const override;
    void SetOutlineColor(std::string const& outlideColor);
    virtual void AppendProperties(std::ostream& strm) const = 0;

private:
    std::string m_color;
    std::string m_outlineColor;
};

• [x] Зачем две переменные для хранения цвета?

[alexey-malov]

• [x] В методе ToString для фигур с заливкой нет цвета заливки

[alexey-malov]

class CRectangle final : public CSolidShape
{
    ...
    std::string m_outlineColor;
    std::string m_fillColor;
};

• [x] Эти поля уже в базовых классах. Незачем хранить их дубликат. У других фигур то же самое

[alexey-malov]

        auto max = std::max_element(
            arr.begin(), arr.end(),
            [](std::shared_ptr<IShape> a, std::shared_ptr<IShape> b) {
                return a->GetArea() < b->GetArea();
            });

• [ ] в лямбду передавать a и b по константой ссылке или по универсальной (auto&&) ссылке

[alexey-malov]

• [ ] Почему в CSolidShape метод AppendProperties объявлен публичным? (его можно вообще не объявлять)