Посоветуйте, как прорефакторить код

@Fenix1123581321 · Регистрация: 24.11.2014

Author24 — интернет-сервис помощи студентам

Нужно создать абстрактный класс.
ОТ него 3 дочерные - фигуры - треугольник, прямоугольник и элипс.
нужно ввести N точек (N>=3) и вывести площадь и принадлежность введеных точек для каждой фигуры
потом отсортировать в по возрастанию кол. точек, пренадлежащих фигурам.
Нужно оптимизировать его
мне он очень не нравиться , опыта сильного не имею , недавно начал учить плюсы.
Нужно ди отдельно создавать хедер на классы и реализации методов ??
Посоветуйте, как сделать код умнее и более правильным

C++

#include <iostream>
#include <conio.h>
#include <iomanip>
 
const double pi = 3.14159;
class IFlatFigure                             /*Abstract base class*/
{
public :
    IFlatFigure(){};
    virtual void Print() = 0;
    virtual double Area() = 0;
    virtual void BelongCheck(int , int) = 0;
    virtual void ShowN() = 0;
    virtual int GetN() = 0;
 
protected :
    int x1;
    int y1;
    int n;
};
 
class Triangle : public IFlatFigure
{
public :
    
    Triangle() : IFlatFigure(){ };
    Triangle(int x1, int y1, int x2, int y2, int x3, int y3)
    {
        Triangle::x1 = x1;
        Triangle::y1 = y1;
        Triangle::x2 = x2;
        Triangle::y2 = y2;
        Triangle::x3 = x3;
        Triangle::y3 = y3;
        Triangle::n = 0;
    }
    void Print()
    {
        std::cout << "Name : Triangle \n" << "Area : " << Area() << std::endl;
    }
    double Area()
    {
        double S = ((x1 - x3)*(y2 - y3) - (x2 - x3)*(y1 - y3));
        return 0.5*fabs(S);
    }
    void BelongCheck(int x0 , int y0)
    {
        double Temp1 = (x1 - x0) * (y2 - y1) - (x2 - x1) * (y1 - y0);
        double Temp2 = (x2 - x0) * (y3 - y2) - (x3 - x2) * (y2 - y0);
        double Temp3 = (x3 - x0) * (y1 - y3) - (x1 - x3) * (y3 - y0);
        if (((Temp1 > 0)&(Temp2 > 0)&(Temp3 > 0)) || ((Temp1 < 0)&(Temp2 < 0)&(Temp3 < 0)))
        {
            std::cout << "Point (" << x0 << "," << y0 << ") belong to Triangle." << std::endl;
            n++;
        }
        else 
        if (!(Temp1*Temp2*Temp3))
        {
            std::cout << "Point (" << x0 << "," << y0 << ") lies on one of the Triangle's lines." << std::endl;
            n++;
        }
            
        else 
            std::cout << "Point (" << x0 << "," << y0 << ") don't belong to this Triangle." << std::endl;
    }
    void ShowN()
    {
        std::cout << "n = " << n << std::endl;
    }
    int GetN()
    {
        return n;
    }
protected :
    int x2;
    int y2;
    int x3;
    int y3;
    
};
class Rectangle : public Triangle
{
public :
    Rectangle() : Triangle(){};
    Rectangle(int x1, int y1, int x2, int y2, int x3, int y3) : Triangle(x1, y1, x2, y2, x3, y3){};
    void Print()
    {
        std::cout << "Name : Rectangle \n" << "Area : " << Area() << std::endl;
    }
    double Area()
    {
        return sqrt(pow((x2 - x1), 2) + pow((y2 - y1), 2))*sqrt(pow((x3 - x1), 2) + pow((y3 - y1), 2));
    }
    void BelongCheck(int x0, int  y0)
    {
        if ((y0 > y1)&(y0 < y2)&(x0 > x1)&(x0 < x3))
        {
            std::cout << "Point (" << x0 << "," << y0 << ") belong to Rectangle." << std::endl;
            n++;
        }
        else
        if (((y0 > y1)&(y0 < y2)&(x0 == x1)) || ((x0 > x1)&(x0 < x3)&(y0 == y1)))
        {
            std::cout << "Point (" << x0 << "," << y0 << ") lies on one of the Rectangle's lines." << std::endl;
            n++;
        }
        else
            std::cout << "Point (" << x0 << "," << y0 << ") don't belong to Rectangle." << std::endl;
    }
    
};
class Elipse : public IFlatFigure
{
public:
    Elipse() : IFlatFigure(){};
    Elipse(int x1, int y1, int Rx , int Ry)
    {
        Elipse::x1 = x1;
        Elipse::y1 = y1;
        Elipse::Rx = Rx;
        Elipse::Ry = Ry;
        Elipse::n = 0;
    }
    void Print()
    {
        std::cout << "Name : Elipse \n" << "Area : " << Area() << std::endl;
    }
    double Area()
    {
        return pi*Rx*Ry;
    }
    void BelongCheck(int x0, int y0)
    {
        std::cout << "Error ;))" << std::endl;
    }
    void ShowN()
    {
        std::cout << "n = " << n << std::endl;
    }
    int GetN()
    {
        return n;
    }
protected :
    int Rx;
    int Ry;
};
struct Point
{
    int x;
    int y;
};
 
 
 
int main()
{
    int N;
    std::cout << "Enter the number of points you want to check (N >= 3) -----> ";
    std::cin >> N;
    while (N < 3)
    {
        std::cout << "Try again \n ";
        std::cout << "Enter the number of points you want to check (N >= 3) -----> ";
        std::cin >> N;
    }
    
    Point *mass = new Point[N];
    for (int i(0); i < N; i++)
    {
        std::cout << "\n" << "Enter the " << i + 1 << "'th point (x , y)";
        std::cin >> mass[i].x >> mass[i].y;
    }
    IFlatFigure *pmass[3];
    pmass[0] = new Triangle(0, 0, 0, 3, 5, 0);
    pmass[1] = new Rectangle(0, 0, 0, 4, 5, 0);
    pmass[2] = new Elipse(0, 0, 3, 4);
 
    pmass[0]->Area();
    pmass[0]->Print();
    for (int i(0); i < N; i++)
    {
        pmass[0]->BelongCheck(mass[i].x, mass[i].y);
    }
    pmass[0]->ShowN();
    
    pmass[1]->Area();
    pmass[1]->Print();
    for (int i(0); i < N; i++)
    {
        pmass[1]->BelongCheck(mass[i].x, mass[i].y);
    }
    pmass[1]->ShowN(); //----------------------------------------------------------------------------------------------------------------------
    
    pmass[2]->Area();
    pmass[2]->Print();
    for (int i(0); i < N; i++)
    {
        pmass[2]->BelongCheck(mass[i].x, mass[i].y);
    }
    pmass[2]->ShowN();
 
    std::cout << "-------------------------------------SORTED----------------------------------" << std::endl;
    
        if (pmass[0]->GetN() > pmass[1]->GetN())
            {
                IFlatFigure *Temp = pmass[0];
                pmass[0] = pmass[1];
                pmass[1] = Temp;
                
                //swap(pmass[i], pmass[j]);
            }
        if (pmass[0]->GetN() > pmass[2]->GetN())
        {
            IFlatFigure *Temp = pmass[0];
            pmass[0] = pmass[2];
            pmass[2] = Temp;
            
        }
        if (pmass[1]->GetN() >
 pmass[2]->GetN())
        {
            IFlatFigure *Temp = pmass[1];
            pmass[1] = pmass[2];
            pmass[2] = Temp;
            
        }
    for (int i = 0; i < 3; ++i)
    {
        pmass[i]->ShowN();
        
    }
    _getch();
}

Для класса Elipse еще не реализовал ф-цию.
Нужно работать динамически.

Tulosba · 05.05.2015, 18:06

Сообщение от Fenix1123581321

Нужно ди отдельно создавать хедер на классы и реализации методов ??

В общем случае - да. Для целей демонстрации - лучше наоборот всё иметь в одном файле. Так проще проверять код.
Замечания по коду:
1. Для IFlatFigure нужен пустой виртуальный деструктор. А явный конструктор не нужен вовсе (в данной интерпретации).
2. Функции, не изменяющие логическое состояние объекта, должны быть объявлены константными. Например:

C++

1	virtual void Print() const = 0;

3. Неконстантные данные должны быть только в private секции. Или только в public если объект работает как структура, но ни в коем случае не в protected. Для данных в private секции следует написать методы доступа set/get по необходимости.
4. Не следует без надобности мешать сишный и плюсовый код. В частности, вместо fabs() следует использовать std::abs().
5. Рекомендуется явно указывать на виртуальность функции в производном классе (см. пример далее в п.6).
6. Если поддерживается Стандарт C++11 рекомендуется в виртуальные функции в производных классах снабжать спецификатором override. Например (с учетом п.2):

C++

1	virtual void Print() const override; // в производных классах

7. Прямоугольник не является треугольником, т.е. код ниже ошибочен:

C++

1	class Rectangle : public Triangle

8. В main() нет освобождения памяти для динамически созданных элементов. Но даже если бы оно было, оно работало бы неправильно из-за проблем п.1.

@Fenix1123581321 · 05.05.2015, 18:36 **[ТС]**

Tulosba, спасибо огромное)
Еще вопросы
1. Зачем деструктор, когда он пустой?

Сообщение от Tulosba

6. Если поддерживается Стандарт C++11 рекомендуется в виртуальные функции в производных классах снабжать спецификатором override. Например (с учетом п.2):
Код C++
1
virtual void Print() const override; // в производных классах

прошу обьяснить, заче делать это явно?

Сообщение от Tulosba

Прямоугольник не является треугольником, т.е. код ниже ошибочен:
Код C++
1
class Rectangle : public Triangle

так просто економиться пам'ять , но теряеться логичность. Вот не знаю, что приоритетней.

Сообщение от Tulosba

В main() нет освобождения памяти для динамически созданных элементов. Но даже если бы оно было, оно работало бы неправильно из-за проблем п.1.

я пробовал и dekete[]Temp и delete Temp - никак не получлось. Это через деструктор ????

Tulosba · 05.05.2015, 19:04

Сообщение от Fenix1123581321

Зачем деструктор, когда он пустой?

Ключевое слово тут - "виртуальный". Ты создаешь объект производного класса и записываешь его адрес в указатель на базовый класс (строки 175-177). Дальше используешь функции, объявленные в базовом классе виртуальными (Print, Area и пр.). Пока всё работает хорошо. Но когда возникнет необходимость удалить созданные объекты (т.е. вызвать delete для каждого), будет вызван деструктор только базового класса (тот класс, который указан в строке 174). Это наиболее вероятный исход. Но по Стандарту вызов delete на объекте производного класса через указатель на базовый при отсутствии виртуального деструктора в базовом классе приводит вовсе к непредсказуемым последствиям (Undefined behaviour).
Для лучшего понимания, можно провести аналогию с функциями: если функция не виртуальная - вызывается тот вариант, который связан со статическим типом указателя, а если виртуальная - то с динамическим.

Сообщение от Fenix1123581321

прошу обьяснить, заче делать это явно?

override следует использовать для лучшего контроля за переопределенными виртуальными функциями. Пример:

C++

class B {
 virtual int f() { return 42; } 
};
class D : public B {
  virtual int f() const /* override */ { return 100500; } 
};

Без использования override код собирается (но фактически в D существует 2 разные функции f: одна константная, а другая - нет), однако, скорее всего программист хотел переопределить виртуальную функцию из базового класса, а не создать параллельно новую.
При добавлении override (т.е. явное указание компилятору, что переопределяется виртуальная функция из базового класса) компилятор ругнется соответствующей ошибкой:

C++

1 2	error: 'virtual int D::f() const' marked override, but does not override virtual int f() const override { return 100500; }

Сообщение от Fenix1123581321

так просто економиться пам'ять , но теряеться логичность. Вот не знаю, что приоритетней.

В первую очередь логика. Если она не будет правильной, код не возможно будет поддерживать и развивать дальше. Когда программа работает неправильно - не важно насколько она быстро работает и мало жрет памяти.

Сообщение от Fenix1123581321

я пробовал и dekete[]Temp и delete Temp - никак не получлось. Это через деструктор ????

Добавить виртуальный деструктор в базовый класс и после этого дописать в main():

C++

1
2
3

delete pmass[0];
delete pmass[1];
delete pmass[2];

Лучше через цикл. Аналогичные действия для всех остальных динамически (через new) создаваемых объектов.

@Fenix1123581321 · 05.05.2015, 19:55 **[ТС]**

Tulosba, уже вас своею тупостю мучать не буду, огромное спасибо за помощь))
Единственное, что хочу спросить :
По какой литературе вы учились ?? Посоветуйте, пожалуйста, наиболее понятную и обьемную литературу.
Я очень плаваю у битах, битовых представлениях чисел, битовых операциях. За что сначала взяться, чтобы структурно и последовательно изуччить косяки.
И самое главное, могу ли я вам иногда задавать вопросы, с которыми у меня будут серьезные проблемы ??
П.С. , я сам не русский, поэтому, пускай мои глупые ошыбки никак не повлияют на ваше впечатление обо мне.

Tulosba · 05.05.2015, 20:38

Сообщение от Fenix1123581321

Посоветуйте, пожалуйста, наиболее понятную и обьемную литературу.

Список литературы можно посмотреть тут.

Сообщение от Fenix1123581321

могу ли я вам иногда задавать вопросы, с которыми у меня будут серьезные проблемы ??

Безусловно. Для этого форум и существует.

@Fenix1123581321 3 / 3 / 0 Регистрация: 24.11.2014 Сообщений: 40
	05.05.2015, 18:36 [ТС]	3
	Tulosba, спасибо огромное) Еще вопросы 1. Зачем деструктор, когда он пустой? Сообщение от Tulosba 6. Если поддерживается Стандарт C++11 рекомендуется в виртуальные функции в производных классах снабжать спецификатором override. Например (с учетом п.2): Код C++ 1 virtual void Print() const override; // в производных классах прошу обьяснить, заче делать это явно? Сообщение от Tulosba Прямоугольник не является треугольником, т.е. код ниже ошибочен: Код C++ 1 class Rectangle : public Triangle так просто економиться пам'ять , но теряеться логичность. Вот не знаю, что приоритетней. Сообщение от Tulosba В main() нет освобождения памяти для динамически созданных элементов. Но даже если бы оно было, оно работало бы неправильно из-за проблем п.1. я пробовал и dekete[]Temp и delete Temp - никак не получлось. Это через деструктор ???? 0

@Fenix1123581321 3 / 3 / 0 Регистрация: 24.11.2014 Сообщений: 40
	05.05.2015, 19:55 [ТС]	5
	Tulosba, уже вас своею тупостю мучать не буду, огромное спасибо за помощь)) Единственное, что хочу спросить : По какой литературе вы учились ?? Посоветуйте, пожалуйста, наиболее понятную и обьемную литературу. Я очень плаваю у битах, битовых представлениях чисел, битовых операциях. За что сначала взяться, чтобы структурно и последовательно изуччить косяки. И самое главное, могу ли я вам иногда задавать вопросы, с которыми у меня будут серьезные проблемы ?? П.С. , я сам не русский, поэтому, пускай мои глупые ошыбки никак не повлияют на ваше впечатление обо мне. 0

Tulosba :) 4773 / 3267 / 497 Регистрация: 19.02.2013 Сообщений: 9,046
	05.05.2015, 20:38	6
	Сообщение от Fenix1123581321 Посоветуйте, пожалуйста, наиболее понятную и обьемную литературу. Список литературы можно посмотреть тут. Сообщение от Fenix1123581321 могу ли я вам иногда задавать вопросы, с которыми у меня будут серьезные проблемы ?? Безусловно. Для этого форум и существует. 0

Посоветуйте, как прорефакторить код

Решение