patterncppMinor
Calculate area and perimeter for shapes: polygon, circle, rectangle and triangle
Viewed 0 times
circlerectangleperimetertriangleshapesforcalculateandpolygonarea
Problem
This is my project on inheritance and polymorphism. I have the main file from my teacher and built the project base on it. My base class is
main.cpp
Shape.h
CIRCLE.H
Circle.cpp
Polygon.h
Polygon.cpp
```
#include "Polygon.h"
#include "Shape.h"
#include "
Shape. Polygon and Circle inherit from Shape. Triangle and Rectangle inherit from Polygon. All of the class use the class Point. The project is running and working I am not sure that the constructors and destructors are working.main.cpp
#include "Point.h"
#include "Shape.h"
#include "Triangle.h"
#include "Circle.h"
#include "Polygon.h"
#include "Rectangle.h"
#include
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 getArea() getPerim() << std::endl;
for (int i = 0; i < 3; ++i)
delete shapes[i];
return 0 ;
}Shape.h
#ifndef Shape_h
#define Shape_h
#include "Point.h"
#include
class Shape
{
public:
virtual double getArea()const=0;
virtual double getPerim() const=0;
virtual ~Shape(){};
};
#endiCIRCLE.H
#ifndef Circle_h
#define Circle_h
#include "Point.h"
#include "Shape.h"
class Circle : public Shape
{
public:
Circle(const Point &l1,int radius);
double getArea()const;
double getPerim()const;
private:
int r;
Point p;
};
#endiCircle.cpp
#include "Circle.h"
#include "Point.h"
#include
#define M_PI 3.14159265358979323846
Circle::Circle(const Point &l1,int radius)
{
r=radius;
p=l1;
}
double Circle::getArea()const
{
return r*r*M_PI;
}
double Circle::getPerim ()const
{
return 2*M_PI*r;
}Polygon.h
#ifndef Polygon_h
#define Polygon_h
#include "Shape.h"
class Polygon :public Shape
{
public:
double getDist(const Point &p1,const Point &p2);
//double getPerim() const{std::cout<<" ";return 0 ;} no need
//double getArea() const {std::cout<<" ";return 0 ;}
};
#endifPolygon.cpp
```
#include "Polygon.h"
#include "Shape.h"
#include "
Solution
General
In many places, your formatting and alignment are inconsistent. You also have typos in your comments and even a couple of typos in the
Adopt a consistent style—whatever it may be—and be disciplined about using it. Use a consistent number of blank lines between functions (unless you are separating them for some reason, and then use a consistent multiple of your normal number of blank lines), be consistent in whether or not you put spaces around operators, be consistent in your indentation, etc.
I also recommend that you outdent the accessibility specifiers (e.g.,
Otherwise, you have a nice solid start here. You appear to have a lot of the basic idioms down, including include guards, virtual functions, const-qualified member functions, and so on. However, you are not using member initialization lists, and I believe that you should. It has no effect on efficiency in this case, since all of your members are POD types, but it is nevertheless a good habit to get into.
Aside from these general comments, I have some further suggestions for improvements that I will present for each class implementation to hopefully make them more manageable. Even if I do not call something out again the next time that it appears, please try to carry knowledge forward!
Point class
-
As mentioned above, this class definition needs to be better formatted. This first time, I'll give an example of what I think good formatting would look like:
Notice how I've standardized on the number of blank lines, outdented my accessibility specifiers, broke member functions up into logical groups, and so forth. This is significantly more readable.
The code file needs the same treatment applied. Also, I recommend placing documentation comments above the function definition, rather than out to the side. This makes things look much neater, especially where you have functions with multiple parameters (or parameters with long names). But consider whether you really need comments for these functions at all! It should be obvious by looking at the name that
-
If you're going to use your current error-handling strategy, you should report them via
If you didn't want to introduce the complexity of exceptions and feel that you can assign reasonable "default"/"fallback" values, then you could simply assert upon receiving an invalid parameter. This would allow the caller to fix their bug in a debugging build, without introducing any error-reporting overhead in release builds. This is more similar to what you have now:
-
In keeping with the "separation of concerns" argument presented above, I would not have a
Of course, since this method doesn't actually require access
In many places, your formatting and alignment are inconsistent. You also have typos in your comments and even a couple of typos in the
#endif portion of the header include guards. All of this needs to be cleaned up because it looks sloppy and makes the code harder to read.Adopt a consistent style—whatever it may be—and be disciplined about using it. Use a consistent number of blank lines between functions (unless you are separating them for some reason, and then use a consistent multiple of your normal number of blank lines), be consistent in whether or not you put spaces around operators, be consistent in your indentation, etc.
I also recommend that you outdent the accessibility specifiers (e.g.,
public:, private:, etc.) in a class definition, and that you begin your comments with a leading space for readability. I would further recommend that you always use braces around if/else blocks, even if you only have one statement there at the moment. When you come along later and add code, it is too easy to forget to add the braces and introduce a bug.Otherwise, you have a nice solid start here. You appear to have a lot of the basic idioms down, including include guards, virtual functions, const-qualified member functions, and so on. However, you are not using member initialization lists, and I believe that you should. It has no effect on efficiency in this case, since all of your members are POD types, but it is nevertheless a good habit to get into.
Aside from these general comments, I have some further suggestions for improvements that I will present for each class implementation to hopefully make them more manageable. Even if I do not call something out again the next time that it appears, please try to carry knowledge forward!
Point class
-
As mentioned above, this class definition needs to be better formatted. This first time, I'll give an example of what I think good formatting would look like:
#ifndef Point_h
#define Point_h
class Point
{
public:
Point();
Point(int x, int y);
int getX() const;
int getY() const;
void setX(int x);
void setY(int y);
void print() const;
private:
int x, y;
};
#endifNotice how I've standardized on the number of blank lines, outdented my accessibility specifiers, broke member functions up into logical groups, and so forth. This is significantly more readable.
The code file needs the same treatment applied. Also, I recommend placing documentation comments above the function definition, rather than out to the side. This makes things look much neater, especially where you have functions with multiple parameters (or parameters with long names). But consider whether you really need comments for these functions at all! It should be obvious by looking at the name that
Point::Point is a constructor for class Point, so you can omit that. And "function update X" barely even makes sense. If you're going to write documentation comments, make them meaningful and professional looking. If they just restate things that are obvious from the name of the function, leave them off!-
If you're going to use your current error-handling strategy, you should report them via
std::cerr, rather than std::cout. But I don't think this is the best approach. In C++, you will generally want to throw an exception when you are passed an invalid parameter that you do not know how to deal with. The caller can then catch this exception and handle it however they want, including displaying an error message. This better preserves a division of responsibility: what if I want to use your Point class in a GUI or remote application where the output from std::cerr is not visible? If you used an exception-based error-handling strategy, I could catch the exception and present the error to the user however I wanted.void Point::setX(int x)
{
if (x x = x;
}
}If you didn't want to introduce the complexity of exceptions and feel that you can assign reasonable "default"/"fallback" values, then you could simply assert upon receiving an invalid parameter. This would allow the caller to fix their bug in a debugging build, without introducing any error-reporting overhead in release builds. This is more similar to what you have now:
void Point::setX(int x)
{
if (x x = 0;
}
else
{
this->x = x;
}
}-
In keeping with the "separation of concerns" argument presented above, I would not have a
print method as part of my Point class. A Point object should not know how to "print" itself. That's far beyond its level of abstraction. If you wanted to facilitate printing a Point object, you could simply define a toString method:// Obtain a formatted string that contains the x and y coordinates of this point.
std::string Point::toString() const
{
std::stringstream ss;
ss x
y;
return ss.str();
}Of course, since this method doesn't actually require access
Code Snippets
#ifndef Point_h
#define Point_h
class Point
{
public:
Point();
Point(int x, int y);
int getX() const;
int getY() const;
void setX(int x);
void setY(int y);
void print() const;
private:
int x, y;
};
#endifvoid Point::setX(int x)
{
if (x < 0)
{
throw std::out_of_range("Invalid value for x");
}
else
{
this->x = x;
}
}void Point::setX(int x)
{
if (x < 0)
{
assert(!"Invalid value for x; setting to default.");
this->x = 0;
}
else
{
this->x = x;
}
}// Obtain a formatted string that contains the x and y coordinates of this point.
std::string Point::toString() const
{
std::stringstream ss;
ss << "x: " << this->x
<< " "
<< "y: " << this->y;
return ss.str();
}std::string to_string(const Point& pt)
{
std::stringstream ss;
ss << "x: " << this->x
<< " "
<< "y: " << this->y;
return ss.str();
}Context
StackExchange Code Review Q#150489, answer score: 9
Revisions (0)
No revisions yet.