4
\$\begingroup\$

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(x<0)
    {
        cout << "Invalid value for x, Setting to default\n";
        this->x=0;
    }
    else
        this->x=x;
}

void Point::setY(int y)
{
    if(y<0)
    {
        cout << "Invalid value for y, Setting to default\n";
        this->y=0;
    }
    else
        this->y=y;
}

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

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

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




class Rectangle
{
public:

Rectangle(int l, int u, int w, int h, int color);
int getColor() const;
void setColor(int color);
bool contains(const Point &p) const;
void print() const;

friend class Grid ; // is there any other way it can work without friend ?

private:
const Point topLeft, bottomRight;
int color;
};



#include <iostream>
#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<=bottomRight.x && p.y>=topLeft.y && p.y<=bottomRight.y)
        return true;
    else
        return false;

}


void Rectangle::print() const
{
    cout << endl;
    cout<<"Rectangle top-left=("<<topLeft.x<<","<<topLeft.y<<")"<<"bottom-right=("<<bottomRight.x<<","<<bottomRight.y<<")"<<"color="<<color;
    cout << endl;
}






class Grid
{

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

~Grid()
{
  //cout <<"Destroy " ;
  delete []recs;  // not sure if it is the right place

}


private:
int count;
Rectangle **recs;
};




Grid::Grid(int tileW, int tileH, int width, int height, int color)  // האם ניתן להניח את תקינות הקלט ? 
{
  int i ,j; 
  count=height*width;
  recs=new Rectangle* [count];

  for(i=0;i<height;i++)
   for(j=0;j<width;j++)
    {
        recs[j+(i*width)]=new Rectangle (j*tileW,tileH*i,tileW,tileH,color);
    }
}

Rectangle& Grid::getRectAt(const Point &p)
{
    Rectangle *r; 
    for(int i=0;i<count;i++)     
        if(recs[i]->contains(p) )
        {
          cout<<"match";
          cout << endl;
          return *(recs)[i];
        }
    cout<<"No mach";
    cout << endl;
    return *(recs)[0];
}

void Grid::print() const 
{

 for (int i=0;i<count;i++)
  {
      recs[i]->print();

  } 

}


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(); 
}
\$\endgroup\$
3
\$\begingroup\$
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<Rectangles>. 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<Rectangle> 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.

\$\endgroup\$
  • \$\begingroup\$ I got your point about namespace std and fiend classsses ,I didn't understand how to use vector ? . About the destructor as it's part of the assignment and I must implement it . Can you show how to write it ? Can i write the destructor in the cpp file ? \$\endgroup\$ – benz Nov 23 '16 at 18:35
  • \$\begingroup\$ Can you show how to write the destructor ? \$\endgroup\$ – benz Nov 25 '16 at 7:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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