tags:

views:

2297

answers:

4

Hi,

I often find myself in a situation where I am facing multiple compilation/linker errors in a C++ project due to some bad design decisions (made by someone else :) ) which lead to circular dependencies between C++ classes in different header files. But fortunately(?) this doesn't happen often enough for me to remember the solution to this problem for the next time it happens again. So for the purposes of easy recall in the future I am going to post a representative problem and a solution along with it (I hope this is not against the rules of stackoverflow). Better solutions are of-course welcome.

//A.h
class B;
class A
{
    int _val;
    B *_b;
public:

    A(int val)
        :_val(val)
    {
    }

    void SetB(B *b)
    {
        _b = b;
        _b->Print();//ERROR: C2027: use of undefined type 'B'
    }

    void Print()
    {
        cout<<"Type:A val="<<_val<<endl;
    }
};

//B.h
#include "A.h"
class B
{
    double _val;
    A* _a;
public:

    B(double val)
        :_val(val)
    {
    }

    void SetA(A *a)
    {
        _a = a;
        _a->Print();
    }

    void Print()
    {
        cout<<"Type:B val="<<_val<<endl;
    }
};

//main.cpp
#include <iostream>

using namespace std;

int _tmain(int argc, _TCHAR* argv[])
{
    A a(10);
    B b(3.14);
    a.Print();
    a.SetB(&b);
    b.Print();
    b.SetA(&a);
    return 0;
}
+5  A: 

You can avoid compilation errors if you remove the method definitions from the header files and let the classes contain only the method declarations and variable declarations/definitions. The method definitions should be placed in a .cpp file (just like a best practice guideline says). The down side of the following solution is (assuming that you had placed the methods in the header file to inline them) that the methods are no longer inlined by the compiler and trying to use the inline keyword produces linker errors.

//A.h
#ifndef A_H
#define A_H
class B;
class A
{
 int _val;
 B* _b;
public:

 A(int val);
 void SetB(B *b);
 void Print();
};
#endif

//B.h
#ifndef B_H
#define B_H
class A;
class B
{
 double _val;
 A* _a;
public:

 B(double val);
 void SetA(A *a);
 void Print();
};
#endif

//A.cpp
#include "A.h"
#include "B.h"

#include <iostream>

using namespace std;

A::A(int val)
:_val(val)
{
}

void A::SetB(B *b)
{
 _b = b;
 cout<<"Inside SetB()"<<endl;
 _b->Print();
}

void A::Print()
{
 cout<<"Type:A val="<<_val<<endl;
}

//B.cpp
#include "B.h"
#include "A.h"
#include <iostream>

using namespace std;

B::B(double val)
:_val(val)
{
}

void B::SetA(A *a)
{
 _a = a;
 cout<<"Inside SetA()"<<endl;
 _a->Print();
}

void B::Print()
{
 cout<<"Type:B val="<<_val<<endl;
}

//main.cpp
#include "A.h"
#include "B.h"

int main(int argc, char* argv[])
{
 A a(10);
 B b(3.14);
 a.Print();
 a.SetB(&b);
 b.Print();
 b.SetA(&a);
 return 0;
}
SDX2000
Fair enough answer... any chance you could modify it to standard C++ rather than VC++ specific stuff (e.g. #pragma once, _tmain, etc)?
workmad3
Sure, as soon as I have some free time on hand but IMO #pragma once is more readable than the #ifndef sentinel/guard.
SDX2000
I agree, but it's not portable :( As this is a 'general' answer, it should really be portable code, rather than specific to a certain compiler/platform.
workmad3
@workmad3: Thanks for the edit.
SDX2000
+5  A: 

Things to remember:

  • This won't work if class A has an object of class B as a member or vice versa.
  • Forward declaration is way to go.
  • Order of declaration matters (which is why you are moving out the definitions).
    • If both classes call functions of the other, you have to move the definitions out.

Read the FAQ: 39.11 39.12 39.13

dirkgently
+7  A: 

The way to think about this is to "think like a compiler".

Imagine you are writing a compiler. And you see code like this.

// file: A.h
class A {
  B _b;
};

// file: B.h
class B {
  B _a;
};

// file main.cc
#include "A.h"
#include "B.h"
int main(...) {
  A a;
}

When you are compiling the .cc file (remember that the cc and not the h is the unit of compilation), you need to allocate space for object A. So, well, how much space then? Enough to store B! What's the size of B then? Enough to store A! Oops.

Clearly a circular reference that you must break.

You can break it by allowing the compiler to instead reserve as much space as it knows about up front - pointers and references, for example, will always be 32 or 64 bits (depending on the architecture) and so if you replaced (either one) by a pointer or reference, things would be great. Lets say we replace A:

// file: A.h
class A {
  // both these are fine, so are various const versions of the same.
  B& _b_ref;
  B* _b_ptr;
};

Now things are better. Somewhat. Main still says:

// file: main.cc
#include "A.h"  // <-- Houston, we have a problem

Include, for all extents and purposes (if you take the preprocessor out) just copies the file into the .cc. So really, the .cc looks like:

// file: partially_pre_processed_main.cc
class A {
  B& _b_ref;
  B* _b_ptr;
};
#include "B.h"
int main (...) {
  A a;
}

You can see why the compiler can't deal with this - it has no idea what B is - it has never even seen the symbol before.

So lets tell the compiler about B.

// main.cc
class B;
#include "A.h"
#include "B.h"
int main (...) {
  A a;
}

This works. It is not great. But at this point you should have an understanding of the circular reference problem and what we did to "fix" it, albeit the fix is bad.

The reason this fix is bad is because the next person including A.h will have to declare B before they can use it and will get a terrible include error. So lets move the declaration into A itself.

// file: A.h
class B;
class A {
  B* _b; // or any of the other variants.
};

And in the B file, at this point, you can just include A directly.

// file: B.h
#include "A.h"
class B {
  // note that this is cool because the compiler knows by this time
  // how much space A will need.
  A _a; 
}

HTH.

Roosh
shouldn't it be `Class B { A _a; };` in the first part of the code?
kobac
+2  A: 

I once solved this kind of problem by moving all inlines after the class definition and putting the #include for the other classes just before the inlines in the header file. This way one make sure all definitions+inlines are set prior the inlines are parsed.

Doing like this makes it possible to still have a bunch of inlines in both(or multiple) header files. But it's necessary to have include guards.

Like this

// File: A.h
#ifndef __A_H__
#define __A_H__
class B;
class A
{
    int _val;
    B *_b;
public:
    A(int val);
    void SetB(B *b);
    void Print();
};

// Including class B for inline usage here 
#include "B.h"

inline A::A(int val) : _val(val)
{
}

inline void A::SetB(B *b)
{
    _b = b;
    _b->Print();
}

inline void A::Print()
{
    cout<<"Type:A val="<<_val<<endl;
}

#endif /* __A_H__ */

...and doing the same in B.h

epatel
Why? I think it's an elegant solution to a tricky problem...when one wants inlines. If one don't want inlines one shouldn't have written the code like it was written from start...
epatel