HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Grid of Rectangles

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gridrectanglesstackoverflow

Problem

I am posting my first project in C++ (and also my first in object-oriented programming), can someone review it and cross check that everything I have coded is alright? If you have any tips to improve my code please tell me. The project is running and working. This is a home assignment and I know there are no default constructor (that is the way the teacher wants it).

The project consists of 3 classes: Point, Rectangle and Grid. Grid builds a dynamic array of Rectangles.

I am not sure when I used destructor, I must have implemented it inside the class?

Are friend classes necessary in this context or is there another way to implement it?

class Point
{

public:
Point(int x, int y);
int getX() const;
int getY() const;
void setX(int x);
void setY(int y);
void print() const;

friend class Rectangle;  

private:
int x, y;
};

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

void Point::setX(int x)
{
    if(xx=0;
    }
    else
        this->x=x;
}

void Point::setY(int y)
{
    if(yy=0;
    }
    else
        this->y=y;
}

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

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

void Point::print() const
{
        cout x y ;
        cout 
#include "Point.h"
#include "Rectangle.h"
using namespace std;

Rectangle::Rectangle(int l=0,int u=0,int w=0,int h=0,int color=0):topLeft(l,u),bottomRight(w+(topLeft.getX()),u+h),color(color)
{   

}

int Rectangle::getColor() const
{
    return color;
}

void Rectangle::setColor(int color)
{
    this->color=color;
}

bool Rectangle::contains(const Point &p) const
{
    int x,y,h,w;
    x=p.getX();
    y=p.getY();

    if(p.x>=topLeft.x && p.x=topLeft.y && p.ycontains(p) )
        {
          coutprint();

  } 

}

void main() 
{   
  Grid g(4, 2, 3, 2, 0);
  cout << "-- before setColor(2) --" << endl; 
  g.print();
  Point p(5, 1);
  g.getRectAt(p).setColor(2);
  cout << "-- after setColor(2) --" << endl; 
  g.print(); 
}

Solution

using namespace std;


This is a bad habit and not one you really want to develop. It leads to various naming conflicts that can be a pain to deal with. Typing std:: every so often isn't that big of a pain.

"dynamic array of Rectangles" should translate into vector. If you don't and use a pointer then you need to take responsibility of cleanup and following the rule of 0/3/5 as needed.

As is you had a leak in your destuctor because you didn't delete the individual Rectangles in the array of pointers.

class Grid
{

    public:
    Grid(int tileW, int tileH, int width, int height, int color);
    Rectangle& getRectAt(const Point &p);
    void print() const;

    // no destructor needed because std::vector gets properly auto-destructed (rule of 0)

    private:
    std::vector recs;

};


Note that the vector will be keeping the Rectangle by value in its internal array.

Friend classes are only needed when the other party needs access to internals that it cannot get otherwise. Making Rectangle a friend of Point is not needed because Point has getters for its fields and Grid access use any private data from Rectangle.

Code Snippets

using namespace std;
class Grid
{

    public:
    Grid(int tileW, int tileH, int width, int height, int color);
    Rectangle& getRectAt(const Point &p);
    void print() const;

    // no destructor needed because std::vector gets properly auto-destructed (rule of 0)

    private:
    std::vector<Rectangle> recs;

};

Context

StackExchange Code Review Q#147875, answer score: 3

Revisions (0)

No revisions yet.