1

I am fairly new to C++ and am working on a personal project. I want to create a vector<Entity*> entitities in C++ where each Entity object is unique. I have a class inside header Entity.h that I want to create. Now Entity takes two member variables:

  1. Rectangle rect - an Object of type Rectangle that has four float variables as member variables (x1, y1, x2, y2) which are the coordinates of two opposite corners of a rectangle
  2. vector<Component*> - a number of different components of Base class type Component with some Derived classes. The code for Class Component looks like so:
/*************************** BASE CLASS **************************/
class Component
{
public:
    virtual ~Component() = default;
    virtual Component* Clone() = 0;
};

/*************************** DERIVED CLASS 1 **************************/
class BasicComponent: public Component
{
private:
    int B= 0;
public:
    BasicComponent* Clone() override {
        return new BasicComponent(B);
    }

    BasicComponent(const int& mB) :B(mB){}
    BasicComponent() :B(0) {}

};

/*************************** DERIVED CLASS 2 **************************/
class AdvancedComponent: public Component
{
private:
    float A = 0.f;
    int iA = 0;
public:
    AdvancedComponent* Clone() override {
        return new AdvancedComponent(A, iA);
    }

    AdvancedComponent(const float& mA, const int& miA) :A(mA),iA(miA) {}
    AdvancedComponent() :A(0.f),iA(0) {}

};

Since I want each Entity in the vector of entities to be unique, that is, have it's own rectangle and components, how should I create the class ?

My question here is, what should the class Entity look like ? Should I create separate CopyConstructor, Assignment Constructor and Destructor for this class ? Also if I want to implement copying one Entity into another (deep copying), is it necessary to have all 3 (Copy, Assignment and Destructor) ?

Rahul Pillai
  • 187
  • 1
  • 9
  • You’ve given the members of `Entity`, right? What’s stopping you from writing the class? – Davis Herring Sep 20 '21 at 01:10
  • Why is `vector` relevant? Is it not enough to say that each `Entity` must have its own copy of the `Rectangle` and `Component`s from which it was constructed? – JaMiT Sep 20 '21 at 04:26
  • @DavisHerring. I already did it but I'm just not sure of my solution (since I'm very new to C++ and normally code on C#. The concept of pointers / addresses confuses me as on C# it's all very easy) – Rahul Pillai Sep 21 '21 at 02:04
  • 1
    @RahulPillai: C# *has* pointers; it just doesn’t have the syntax. – Davis Herring Sep 21 '21 at 02:07
  • @JaMiT. Not sure what you meant. Is it wrong to use ```vector``` instead of ```vector``` ? . I was just not sure about what was more efficient and I just wanted to not lose my entities from out of the heap and so did not want them to be created on the stack. – Rahul Pillai Sep 21 '21 at 02:10
  • 1
    @RahulPillai *"Is it wrong to use `vector` instead of `vector` ?"* -- That's not what I am getting at. You are asking about your code; I am talking about your question. Simpler questions have fewer distractions, hence are more likely to get answered without getting lost on a tangent. What is lost if you drop all mention of vectors **from your question** and just state that each `Entity` must have its own `Rectangle` and `Component`s? (Emphasizing the vector over the requirement makes it seem like maybe objects not in the vector must share rectangles and components.) – JaMiT Sep 21 '21 at 11:09
  • @JaMiT. I see your point now. Let me make it a little more clear: I want to know why pointers are useful over variables when I want to make a clone? Isn't that losing out on the very purpose of pointers ? So I did some reading online and found out that when you create an object using the new keyword, it returns a pointer and stores the object on the heap whereas otherwise it stores it on the stack. So I thought I would lose out on accessing Entities outside of the function where I create it. (Feel free to correct me if I am wrong or educate me. I am here to learn :) ) – Rahul Pillai Sep 21 '21 at 14:38
  • 2
    There are various reasons to use dynamic allocation, but if you don't really _want_ a dynamic lifetime, then in principle you don't need it. However, if you want runtime polymorphism (as you Component subclasses show), it's hard to do without indirection. – Useless Sep 21 '21 at 15:52

2 Answers2

2

My question here is, what should the class Entity look like ? Should I create separate CopyConstructor, Assignment Constructor and Destructor for this class ? Also if I want to implement copying one Entity into another (deep copying), is it necessary to have all 3 (Copy, Assignment and Destructor) ?

The answer to this doesn't really depend on what the Entity looks like, but on what semantics it is intended to have.

You say every Entity is "unique", but every object in C++ (even every int, even when they happen to have the same value) is technically unique. So what do you really mean?

  1. Should an Entity be copyable? Copy-constructing an Entity would mean two Entity objects have the same contents (literally if the component pointers are shallow-copied, and logically if they're deep-copied).

    If not, you probably don't want to write a (or may explicitly delete the) copy constructor and/or copy-assignment operator.

  2. Should an Entity be moveable? Probably yes, since it doesn't violate uniqueness and makes them easier to use efficiently.

    If so, you should make sure it has a move constructor and move-assignment operator (either by writing it or arranging for the compiler to generate useful defaults).

Also if I want to implement copying one Entity into another (deep copying)

That seems to violate your uniqueness constraint, but yes, you'd want a copy constructor and copy-assignment operator for this.

However, best practice is to avoid interleaving resource management with your program logic. So, instead of writing all these, consider having a smart pointer automate it for you. See for comparison the Rule of Zero mentioned in this answer.

In fact, we can illustrate all the reasonable semantics with very little code:

template <typename ComponentPtr>
struct ZeroEntity
{
  Rectangle bound;
  std::vector<ComponentPtr> components;
};

using ShallowCopyZeroEntity = ZeroEntity<std::shared_ptr<Component>>;
using NoCopyOnlyMoveEntity = ZeroEntity<std::unique_ptr<Component>>;
using DeepCopyZeroEntity = ZeroEntity<my::clone_ptr<Component>>;

except for the fact that we still need to write a deep-copying clone_ptr, something like

namespace my {
template <typename T>
class clone_ptr
{
  std::unique_ptr<T> p_;

  std::unique_ptr<T> clone() const { return std::unique_ptr<T>{p_ ? p_->Clone() : nullptr}; }

public:

  using pointer = typename std::unique_ptr<T>::pointer;

  explicit clone_ptr(pointer p) : p_(p) {}

  // copy behaviour is where the cloning happens
  clone_ptr(clone_ptr const& other) : p_(other.clone()) {}
  clone_ptr& operator=(clone_ptr other)
  {
    other.swap(*this);
  }

  // move behaviour (and destructor) generated by unique_ptr
  clone_ptr(clone_ptr&& other) = default;
  clone_ptr& operator=(clone_ptr&&) = default;

  // now write all the same swap, release, reset, operator* etc. as std::unique_ptr
};
}

... and if that looks like a lot, imagine how messy it would have been interleaved with your Entity code instead of collected into one place like this.

Useless
  • 64,155
  • 6
  • 88
  • 132
  • Thanks so much for this! I was just getting used to the very idea of smart pointers so I did not use that in my solution. This project is more of a learning experience for me. I tried implementing the solution without pointers as shown below. I correct myself by saying Entity is unique. I think a better way to say it is they may or may not have multiple components in the vector and different Rectangles. This doesn't really make them unique because I still need to enable some system of being able to clone them. Please let me know if my solution could lead to memory leaks. – Rahul Pillai Sep 21 '21 at 19:48
0

The primary purpose is only to enable copying of the Entity. Does this work ?

#include "Components.h"
#include "Rectangle.h"
#include <vector>
using namespace std;
class Entity
{

public:
    Rectangle& Box;
    vector<Component*>& Components;

    //Default constructor call
    Entity(): Box(*new Rectangle), Components(*new vector<Component*>){}
 
    //Constructor call with parameters
    Entity(Rectangle& B, vector<Component*>& C) : Box(B), Components(C){}

    //Copy constructor that makes Entity copyable
    Entity(const Entity& E) : Box(*new Rectangle(E.Box)), Components(*new vector<Component*>(E.Components.size())) {
     
 
        for (unsigned int i = 0; i < E.Components.size(); i++) {
            Components[i] = E.Components[i]->Clone();
        }
    }

    //Assignment operator constructor
    Entity& operator=(const Entity& E){ 
        //Entity* pE = &E;
        if (this != &E)
        {
            delete& Box;
            delete& Components;
            Box = *new Rectangle(E.Box);
            Components = *new vector<Component*>(E.Components.size());
            for (unsigned int i = 0; i < E.Components.size(); i++) {
                Components[i] = E.Components[i]->Clone();
            }
        }
         return *this;

    }

};

Rahul Pillai
  • 187
  • 1
  • 9
  • 1
    Decide on the semantics of things first. Should the `Components` vector live exactly as long as the Entity? Then make it a regular member. You don't need to dynamically allocate a single vector. – Useless Sep 21 '21 at 22:29
  • 1
    If you _do_ have to dynamically allocate something, don't store just a `&` reference. They're not used to indicate ownership, and a raw pointer at least doesn't hide the code smell. Prefer automatic lifetime first, and if you must dynamically allocate, use smart pointers. Owning raw pointers (which you must manually `delete`) are a distant third choice, and owning references don't appear on the list at all. – Useless Sep 21 '21 at 22:31
  • 1
    The cloning itself will work (if inelegantly), but your `operator=` has zero exception safety: if either `new` or one of the `Clone` calls throws an exception, you've already trashed the left-hand-side `Entity` you were assigning to, and now it can't be used. First write a destructor, then write a non-throwing `swap` (or move assignment), and write the normal copy assignment that copy-constructs a temporary and then swaps it with `*this`. – Useless Sep 21 '21 at 22:37
  • Thanks. That makes so much more sense ! – Rahul Pillai Sep 22 '21 at 23:39