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

alexey-malov commented 5 years ago

[ ] Основное приложение не компилируется

alexey-malov commented 5 years ago

#pragma once
#include "pch.h"
#include "../ishape/ShapeFactory.h"
#include "../ishape/ShapeFactory.cpp"

#include "../ishape/CCircle.cpp"
#include "../ishape/CLineSegment.cpp"
#include "../ishape/CPoint.cpp"
#include "../ishape/CRectangle.cpp"
#include "../ishape/CTriangle.cpp"
#include "../ishape/IShape.cpp"
#include "../ishape/ISolidShape.cpp"

#include "../ishape/IShapeArray.h"
#include "../ishape/IShapeArray.cpp"

[x] Лучше добавить .cpp файлы в проект с тестами, либо сделать статическую библиотеку
[ ] Заголовочные файлы в проекте с тестами должны лежать в фильтре "Файлы заголовков"

alexey-malov commented 5 years ago

class IShape
{
public:
    virtual double GetArea() const = 0;
    virtual double GetPerimeter() const = 0;
    virtual std::string ToString() const = 0;
    virtual uint32_t GetOutlineColor() const;

    virtual ~IShape(){};

protected:
    uint32_t m_outlineColor;

    static uint32_t HexStringToUint(const std::string& string);
    void SetOutlineColorByString(const std::string& string);
};

class ISolidShape : virtual public IShape
{
public:
    virtual uint32_t GetFillColor() const;

    virtual ~ISolidShape(){};

protected:
    uint32_t m_fillColor;

    virtual void SetFillColorByString(const std::string& string);
};

[x] В интерфейсе не следует размещать ничего кроме чисто виртуальных методов. Т.е. данных и обычных методов там размещать не следует.

alexey-malov commented 5 years ago

[x] Не ясно предназначение IShapeArray.

alexey-malov commented 5 years ago

    std::pair<IShape*, IShape*> GetMaxAreaMinPerimeter() const;

[x] pair для хранения этого результата подходит плохо. Сделайте структуру, либо 2 разные функции

alexey-malov commented 5 years ago

[x] Используйте умный указатель std::unique_ptr, тогда не придётся вручную освобождать память.

alexey-malov commented 5 years ago

class ShapeFactory
{
public:
    static IShape* GetShape(const std::string& shapeName);

[x] Зачем здесь класс? Почему нельзя заменить обычной функцией.
[x] Лучше подошло бы имя для функции/метода CreateShape

alexey-malov commented 5 years ago

[x] Классы фигур должны получать не вектор строк, а конкретные параметры нужного типа

alexey-malov commented 5 years ago

2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ccircle.h(22): warning C4250: 'CCircle': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>CCircle.cpp
2>CLineSegment.cpp
2>CPoint.cpp
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ccircle.h(22): warning C4250: 'CCircle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>CLineSegment.cpp
2>CRectangle.cpp
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\crectangle.h(27): warning C4250: 'CRectangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>CPoint.cpp
2>CShape.cpp
1>CRectangle.cpp
2>CSolidShape.cpp
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\crectangle.h(27): warning C4250: 'CRectangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>CShape.cpp
2>CTriangle.cpp
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ctriangle.h(24): warning C4250: 'CTriangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>CSolidShape.cpp
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>ShapeArray.cpp
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ccircle.h(22): warning C4250: 'CCircle': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\crectangle.h(27): warning C4250: 'CRectangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ctriangle.h(24): warning C4250: 'CTriangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>CTriangle.cpp
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ctriangle.h(24): warning C4250: 'CTriangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>main.cpp
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ccircle.h(22): warning C4250: 'CCircle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\crectangle.h(27): warning C4250: 'CRectangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ctriangle.h(24): warning C4250: 'CTriangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(6): error C2039: 'optional': is not a member of 'std'
1>c:\vs2017\vc\tools\msvc\14.15.26726\include\iostream(14): note: see declaration of 'std'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(6): error C2143: syntax error: missing ';' before '<'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(6): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(7): error C2143: syntax error: missing ';' before '{'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(7): error C2447: '{': missing function header (old-style formal list?)
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(21): error C2039: 'optional': is not a member of 'std'
1>c:\vs2017\vc\tools\msvc\14.15.26726\include\iostream(14): note: see declaration of 'std'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(21): error C2143: syntax error: missing ';' before '<'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(21): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(21): error C2086: 'int optional': redefinition
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(6): note: see declaration of 'optional'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(22): error C2143: syntax error: missing ';' before '{'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(22): error C2447: '{': missing function header (old-style formal list?)
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(49): error C3861: 'AskForInput': identifier not found
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\main.cpp(55): error C3861: 'GetShapes': identifier not found
2>ShapeFactory.cpp
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ccircle.h(22): warning C4250: 'CCircle': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\crectangle.h(27): warning C4250: 'CRectangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ctriangle.h(24): warning C4250: 'CTriangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>ShapeArray.cpp
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ccircle.h(22): warning C4250: 'CCircle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\crectangle.h(27): warning C4250: 'CRectangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ctriangle.h(24): warning C4250: 'CTriangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>ShapeFactory.cpp
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ccircle.h(22): warning C4250: 'CCircle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\crectangle.h(27): warning C4250: 'CRectangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ctriangle.h(24): warning C4250: 'CTriangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
1>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>main.cpp
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\csolidshape.h(17): warning C4250: 'CSolidShape': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ccircle.h(22): warning C4250: 'CCircle': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\crectangle.h(27): warning C4250: 'CRectangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\ctriangle.h(24): warning C4250: 'CTriangle': inherits 'CShape::CShape::GetOutlineColor' via dominance
2>c:\teaching\oop-2019\yambakov\oop\lab4\shape\cshape.h(10): note: see declaration of 'CShape::GetOutlineColor'
1>Generating Code...

[ ] В конфигурации debug 64 проект не собирается. Он должен собираться во всех конфигурациях и платформах

alexey-malov commented 5 years ago

[ ] Чтобы избавиться от предупреждения насчёт методов, унаследованных via dominance, переопределите метод в классе, с множественным наследованием и явно вызовите нужный метод родителя

alexey-malov commented 5 years ago

[ ] Данные следует делать приватными

alexey-malov commented 5 years ago

CSolidShape::CSolidShape(uint32_t fillColor, uint32_t outlineColor)
    : m_fillColor(fillColor)
    , CShape(outlineColor)
{
}

[ ] Сперва вызывается конструктор родителя, поэтому его пишите вверху, чтобы код выглядел и работал одинаково

alexey-malov commented 5 years ago

[ ] Из строки можно данные поместить в istringstream, а оттуда читать при помощи оператора >> сразу в переменные. Тогда не нужно makeuint и т.п.

alexey-malov commented 5 years ago

[ ] Подберите имена тестов так, чтобы их имена выглядели как спецификация на тестируемый класс или функцию - посмотрите как сделано в примере с телевизором и презентацию по Test-Drivent Development в группе ВК

ImbaCow / OOP

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