Constructor in implementation versus header

Question

The constructor should, to my knowledge, be defined in the implementation file but I've only been able to find examples with the class inside one main file instead of split into a .h and .cpp file

All I need to know is if my following code is separated in an acceptable manner..

Entity.h:

    using namespace std;

class cEntity {
private:
    /*-----------------------------
    ----------Init Methods---------
    -----------------------------*/
    int *X, *Y;
    int *Height, *Width;

public:
    /*-----------------------------
    ----------Constructor----------
    -----------------------------*/
    cEntity (int,int, int, int);

    /*-----------------------------
    ----------Destructor-----------
    -----------------------------*/
    ~cEntity ();

    /*-----------------------------
    ----------Set Methods----------
    -----------------------------*/

    /*Set X,Y Methods*/
    void setX(int x){*X=x;};
    void setY(int y){*Y=y;};
    void setXY(int x, int y){*X=x; *Y=y;};

    /*Set Height, Width Methods*/
    void setHeight(int x){*Height=x;};
    void setWidth(int x){*Width=x;};
    void setDimensions(int x, int y){*Height=x; *Width=y;};

    /*-----------------------------
    ----------Get Methods----------
    -----------------------------*/

    /*Get X,Y Methods*/
    int getX(){return *X;};
    int getY(){return *Y;};

    /*Get Height, Width Methods*/
    int getHeight(){return *Height;};
    int getWidth(){return *Width;};
};

and Entity.cpp:

#include "Entity.h"


cEntity::cEntity (int x, int y, int height, int width) {
   X,Y,Height,Width = new int;
  *X = x;
  *Y = y;
  *Height = height;
  *Width = width;
}

cEntity::~cEntity () {
  delete X, Y, Height, Width;
}

I would also like to say thanks to everyone for being so helpful, especially on my first question!

Answer 1

cEntity::cEntity (int x, int y, int height, int width) {

is correct

   X,Y,Height,Width = new int;

not so much. That sets Width to a new int, but not the rest. You probably intended:

   X = new int(x);
   Y = new int(y);
   Height = new int(height);
   Width = new int(width);

Note that this method of construction will not work for objects without assignment/copy, like references. For some objects, it's also slower than constructing them in place. As such, the preferred way to construct is like so:

cEntity::cEntity (int x, int y, int height, int width) {
    :X(new int(x))
    ,Y(new int(y))
    ,Height(new int(height))
    ,Width(new int(width))
{}

This is better, but if any exceptions are thrown, you'll have to somehow deallocate the ones that were allocated. Better is to make each of those members a std::unique_ptr<int>, so they'll deallocate themselves and save you many headaches.

Answer 2

Yes, it's OK. However, there is a problem with your constructor and destructor. What your code actually does is allocating one int and your destructor deallocates one int also. Anyway, there is no need to use pointers here. Somewhat better implementation (if we don't use smart pointers), could be:

[Entity.h]

private:
    /*Private fields*/
    int X, Y;
    int Height, Width;

[Entity.cpp]

cEntity::cEntity (int x, int y, int height, int width) {
  X = x;
  Y = y;
  Height = height;
  Width = width;
}
cEntity::~cEntity () {
}

And one more thing. Try to avoid using namespace std; in your header files. If you do, you force those who include your header to use this using statement and it can provoke namespace clashes.

Answer 3

Your separation is fine. The implementations of those functions is wrong, but you've separated them from the declaration suitably. (They don't allocate or free as many objects as you think they do.)

Answer 4

Yes. For the separation at least, that's generally the best way to do it.

As for the actual implementation you have some issues. I am not really sure what you are trying to do with the constructor or if you have the correct data types for the class member variables but something seems off.

Answer 5

Any method defined in the class directly is implicitly inlined, including the constructor.

I.e.

class MyClass  
{  
public:    
    MyClass() {};  
};

defines an inline constructor, which may (or may not) improve your code performance,

Whereas

class MyClass  
{  
public:  
    MyClass();  
};  

MyClass::MyClass()
{
};

is not inlined, and therefore won't have those benefits. Both options are correct C++ though.

Just my 2 cents.

P.S And yes, when you decide to store pointers inside a class in this manner, you open a Pandora box.