How can I avoid including a class stored as a field in a data structure?

Question

Let's say I have a class Squad with a dynamic array of Units. I'm looking for a way to avoid #includeing "Unit.h" into "Squad.h" and users of the latter. The global idea is avoiding #includes leading to other ones - a pretty annoying thing.

I've found a simple solution like this:

// Squad.h
class Unit;
class Squad {
private:
  Unit*  units;
  size_t units_num;
public:
  void do_something_with_units(); // its implementation requires Unit to be a full type
};

// Squad.cpp
#include "Unit.h" // it's fine (and required) in Squad.cpp
void Squad::do_something_with_units() { // implements
}

// squad_user.h
// I want this whole file to be satisfied with only
// a forward-declaration of Unit (coming from Squad.h)
// provided it doesn't interact with Unit directly
void foo() {
  Squad squad;
  squad.do_something_with_units(); // works!
}

(Putting forward-declared Units in a std::vector inside Squad fails because it needs Squad's users to #include Unit themselves, otherwise the vector can't allocate.)

Question number one: is such practice acceptable? Of course, I'm not going to rewrite the guts of std::vector (and others) every time, but making a header with algorithms like template<typename T> T* reallocate(T*, size_t current, size_t needed) for later #includes in .cpp seems bearable.

Question number two: is there a better solution? I know about the pimpl idiom but dislike it for frequent small heap allocations.

By the way, what should I do with this when I need a more complicated data structure like std::unordered_map? Replacing an array isn't hard but there are "worse" cases like this one.

Answer 1

At a guess, a key issue here is that vector is defined to be a useful class, handling several things (like copying) automatically. The cost of full automation is that the value type (Unit) needs to be a complete type when the containing class (Squad) is defined. To remove this cost, some of the automation must be given up. Some things are still automated, but now the programmer needs to be aware of the Rule of Three (or Five). (Full automation morphs this rule into the Rule of Zero, which is trivial to follow.)

The kicker is that any workaround would also require knowledge of the Rule of Three, so they do not end up being better than the vector approach. They might look better initially, but not after all the bugs are fixed. So, no, don't invent complex data structures; stick with vector.

---

is such practice acceptable?

In general, the practice is acceptable if done correctly. Typically, though, it's not warranted. Furthermore, your implementation is unacceptably incomplete.

1. Your Squad class has a raw pointer that is not initialized, allowing it to pick up a garbage value. You need to initialize your data members (especially since they are private, hence only the class can initialize them).

2. Your raw pointer is intended to own the memory to which it points, so your class needs to follow the Rule of Three (or Five). Your class needs to define (or delete) a copy constructor, a copy assignment operator, and a destructor. All of these definitions will (likely) require a full declaration of Unit so you want the definitions in your implementation file alongside do_something_with_units (i.e. not defined inline in the header file).

Is this being nitpicky? Not really. Addressing these omissions leads us to the realm of "not warranted". Let's look at your new class definition.

// Squad.h
class Unit;
class Squad {
private:
  Unit*  units;
  size_t units_num;
public:
  Squad();
  Squad(const Squad &);             // implementation requires Unit to be a full type
  Squad & operator=(const Squad &); // implementation requires Unit to be a full type
  ~Squad();                         // implementation requires Unit to be a full type

  void do_something_with_units();   // implementation requires Unit to be a full type
};

At this point, it would be possible to implement the default constructor in the header file (if it initializes units to nullptr instead of allocating memory), but let's entertain the possibility that it's no big deal to put it in the implementation file along with the other new functions. If you can accept that, then the following class definition works, with similar requirements for the implementation file.

// Squad.h
#include <vector>
class Unit;
class Squad {
private:
  std::vector<Unit> units;
public:
  Squad();                          // implementation requires Unit to be a full type
  Squad(const Squad &);             // implementation requires Unit to be a full type
  Squad & operator=(const Squad &); // implementation requires Unit to be a full type
  ~Squad();                         // implementation requires Unit to be a full type

  void do_something_with_units();   // implementation requires Unit to be a full type
};

I made two changes: the raw pointer and size were replaced by a vector, and the default constructor now requires Unit to be a full type for its implementation. The implementation of the default constructor might be as simple as Squad::Squad() {}, as long as the full definition of Unit is available at that point.

And that is probably what you were missing. The vector approach works if you provide explicit implementations for constructing, destructing, and copying, and if you put those implementations in your implementation file. The definitions might look trivial since the compiler does a lot of work behind the scenes, but those functions are what impose the requirement that Unit be a complete type. Take their definitions out of the header file, and the plan works.

(This is why the commentors were confused about why you thought a vector would not work. Outside the constructor, the times when vector needs Unit to be a complete type are exactly the times when your implementation would need Unit to be a complete type. Your proposed implementation is more work for no additional benefit.)