2
\$\begingroup\$

This is the new code after the changes suggested by @Cody Gray in the last question:

Calculate area and perimeter for shapes: polygon, circle, rectangle and triangle

This is my project on inheritance and polymorphism. I got the main file from my teacher and built the project base on it. My base class is Shape. Polygon and Circle inherit from the Shape class. Triangle and Rectangle inherit from Polygon. All of the classes use the class Point.

Few questions and comments:

  1. I'm still learning what the best code format is, so I didn't format this code yet

  2. After I removed the getDist function from the Polygon class now it's completely empty. Logically it still make sense to use it. Is it ok to leave an empty class in my project?

  3. No matter what I did the build in PI number didn't work and static const either. I changed PI to be local variable but how can I make it work correctly?

  4. I created empty constructors and destructors to every class since my teacher demanded it.

  5. I didn't implement the changes on "main" file because I am not allowed to change it (part of the assignment).

  6. I used the override function but I still don't fully understand what it does.

main.cpp

#include "Point.h"
#include "Shape.h"    
#include "Circle.h"     
#include "Polygon.h"     
#include "Rectangle.h"   
#include "Triangle.h"   
#include <iostream>

int main()
{ 
    Point o(0, 0);
    Point a(0, 1);
    Point b(1, 0);
    Shape *shapes[] = { new Rectangle(a, b), new Triangle(o, a, b), new Circle(o, 1) };

    for (int i = 0; i < 3; ++i) 
      std::cout << i << ") area=" << shapes[i]->getArea() <<" perim=" << shapes[i]->getPerim() << std::endl; 

    for (int i = 0; i < 3; ++i) 
        delete shapes[i]; 

    return 0 ; 
}

Point.h

#ifndef Point_h
#define Point_h

class Point
{
    public:

    Point();
    Point(int x, int y);
    ~Point(){}

    int getX() const;
    int getY() const;

    void setX(const int &x);
    void setY(const int &y);

    void print() const;

    private:

    int x, y;
};

double getDist(const Point &p1, const Point &p2);


#endif 

Point.cpp

#include <iostream>
#include "Point.h"

Point::Point()
{
    setX(0);
    setY(0);
}

Point::Point(int x,int y )
{
 setX(x);
 setY(y);
}

void Point::setX(const int &x) 
{
    if(x<0)
    {
        throw std::out_of_range("Invalid value for X \n");
        this->x=0;
    }
    else
    {
        this->x=x;
    }
}

void Point::setY(const int &y)
{
    if(y<0)
    {
        throw std::out_of_range("Invalid value for Y \n");
        this->y=0;
    }
    else
    {
        this->y=y;
    }
}

int Point::getX() const 
{
    return x;
} 

int Point::getY() const 
{
    return y;
}

void Point::print() const 
{
        std::cout <<"X:"<< this->x <<"  "<<"Y:" << this->y ;
}

double getDist(const Point &p1, const Point &p2)
{
    const double distX        = (p1.getX() - p2.getX());
    const double distXSquared = (distX * distX);
    const double distY        = (p1.getY() - p2.getY());
    const double distYSquared = (distY * distY);
    const double distXY       = std::sqrt(distXSquared + distYSquared);
    return distXY;
}

Shape.h

#ifndef Shape_h
#define Shape_h
#include "Point.h"
#include <iostream>


class Shape
{
  public:
    Shape() {}
    virtual  ~Shape(){};

    virtual double getArea()const=0;
    virtual double getPerim() const=0;
};

#endif 

Circle.h

#ifndef Circle_h
#define Circle_h
#include "Point.h"
#include "Shape.h"



class Circle : public Shape
{
public:

    Circle(const Point &ceneterP,const int &radius);
    ~Circle(){}

    virtual double getArea()const override;
    virtual double getPerim()const override;

private:
    int m_radius;
    Point m_centerP;
};




#endif 

Circle.cpp

#include "Circle.h"
#include "Point.h"
#include <iostream>
#include <cmath>

Circle::Circle(const Point &centerP,const int &radius)
:m_radius(radius)
,m_centerP(centerP)
{   
}

double Circle::getArea()const
{   
    double PI  =3.141592653589793238463;
    return m_radius*m_radius*PI;
}

double Circle::getPerim ()const
{   
    double PI  =3.141592653589793238463;
    return 2*PI*m_radius;
}

Polygon.h

#ifndef Polygon_h
#define Polygon_h
#include "Shape.h"

class Polygon :public Shape
{

};

#endif

Rectangle.h

#ifndef Rectangle_h
#define Rectangle_h
#include "Point.h"
#include "Polygon.h"

class Rectangle:public Polygon 
{
    public:

    Rectangle(const Point &l, const Point &r) ;
    ~Rectangle(){}

    virtual double getArea()const override;
    virtual double getPerim()const override;

    private:
        Point m_topLeft;
        Point m_bottomRight;
        int m_length ,m_width;
};

#endif 

Rectangle.cpp

#include "Rectangle.h"
#include <iostream>

Rectangle::Rectangle(const Point &topLeft, const Point &bottomRight):
m_topLeft(topLeft),
m_bottomRight(bottomRight)
{   

    const Point tempPoint(m_bottomRight.getX(),m_topLeft.getY());
    m_length = getDist(m_topLeft, tempPoint);     
    m_width  = getDist(tempPoint, m_bottomRight);

}

double Rectangle::getArea()const
{

    return m_length*m_width;

}

double Rectangle::getPerim()const
{   

    return (m_length+m_width)*2;

}

Triangle.h

#ifndef Triangle_h
#define Triangle_h
#include "Point.h"
#include "Polygon.h"

class Triangle : public Polygon
{
public:

    Triangle(const Point &vertexA, const Point &vertexB,const Point &vertexC);
    ~Triangle (){}

    virtual double getArea()const override;
    virtual double getPerim()const override;

private:
    Point m_vertexA;
    Point m_vertexB;
    Point m_vertexC;
    double edgeA,edgeB,edgeC;

};
#endif 

Triangle.cpp

#include "Triangle.h"
#include <cmath>
#include <iostream>

Triangle::Triangle(const Point &vertexA, const Point &vertexB,const Point &vertexC)
 :m_vertexA(vertexA)
 ,m_vertexB(vertexB)
 ,m_vertexC(vertexC)
 , edgeA(getDist(m_vertexA, m_vertexB))   
 , edgeB(getDist(m_vertexB, m_vertexC))
 , edgeC(getDist(m_vertexA, m_vertexC))
{   
}

double Triangle::getArea()const
{   
    double area,temp;
    temp= (edgeA + edgeB + edgeC)*
          (edgeA + edgeB - edgeC)*
          (edgeA - edgeB + edgeC)*
          (-edgeA + edgeB + edgeC); 
    area = sqrt(temp);              
    return area*0.25 ;
}

double Triangle::getPerim ()const
{   
    return edgeA+edgeB+edgeC;
}
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

A few thoughts...

Unreachable Code

When you throw an exception, execution jumps to the associated catch block (if one exists, or the program crashes). Where you've introduced your throws in your Point class to replace logging you've left subsequent lines intact:

throw std::out_of_range("Invalid value for Y \n");
this->y=0;

The line this->y=0 will never be executed, so should be removed, it's just confusing.

Include Order

I always include system includes, before project includes. This is partly because I don't put includes in my header files, but it's also because I know that the system headers shouldn't depend on my headers, but my headers may depend on the system headers. Essentially, include files in the order the compiler is likely to need them. You should also pick a style. If you're going to include dependencies from headers then don't also include them from your cpp files. At the moment 'Circle.cpp' includes 'Point.h' which is also included from 'Circle.h'. When you're working with very large projects even this small duplication can add up to noticeable compile time differences.

Polygon

Your polygon class feels like an artificial abstraction. If you're not planning on adding any functionality to it, and you don't need something that can refer to 'Polygon's, rather than just 'Shape's then I would remove it.

Naming

You mostly use descriptive names, however should 'tempPoint'

const Point tempPoint(m_bottomRight.getX(),m_topLeft.getY());

Be 'topRight'? It makes it easier to understand what you're doing...

You're also being inconsistent with your use of member field naming. Triangle has 'm_vertexA' etc, but also has 'edgeA'. Pick a naming style and be consistent. If you're going to prefix with an 'm_' always prefix it.

\$\endgroup\$
0
\$\begingroup\$

Some suggestions:

\$\endgroup\$
1
  • \$\begingroup\$ The last idea runs counter to the point of the question, the OP wants to calculate PI, not just use it. \$\endgroup\$ Dec 27, 2016 at 8:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.