How can I handle circular #include calls in a template class header?

Question

Related to Error "Unterminated conditional directive" in cross-referencing headers

I have a Serializable template class:

serializable.h

#pragma once
#ifndef SERIALIZABLE_H
#define SERIALIZABLE_H
#include "Logger.h"
#include <string>
#include <boost/property_tree/ptree.hpp>
#include <boost/property_tree/json_parser.hpp>
#include <boost/exception/diagnostic_information.hpp>
#include <boost/exception_ptr.hpp>

template<class T>
class Serializable {
public:
    static bool Deserialize(Serializable<T>* object, std::string serializedObject) {
        try {
            return object->SetValuesFromPropertyTree(GetPropertyTreeFromJsonString(serialized));
        } catch (...) {
            std::string message = boost::current_exception_diagnostic_information();
            Logger::PostLogMessageSimple(LogMessage::ERROR, message);
            std::cerr << message << std::endl;
        }
    }
private:
    static boost::property_tree::ptree GetPropertyTreeFromJsonString(const std::string & jsonStr) {
        std::istringstream iss(jsonStr);
        boost::property_tree::ptree pt;
        boost::property_tree::read_json(iss, pt);
        return pt;
    }
}
#endif // SERIALIZABLE_H

But the problem is that the Logger class uses a LogMessage object which inherits from Serializable (using CRTP).

Logger.h

#pragma once
#ifndef LOGGER_H
#define LOGGER_H
#include "LogMessage.h"

class Logger {
public:
    static void PostLogMessageSimple(LogMessage::Severity severity, const std::string & message);
}
#endif // LOGGER_H

LogMessage.h

#pragma once
#ifndef LOGMESSAGE_H
#define LOGMESSAGE_H
#include "serializable.h"

class LogMessage : public Serializable<LogMessage> {
public:
    enum Severity {
        DEBUG,
        INFO,
        WARN,
        ERROR
    };
private:
    std::string m_timestamp;
    std::string m_message;

    friend class Serializable<LogMessage>;
    virtual boost::property_tree::ptree GetNewPropertyTree() const;
    virtual bool SetValuesFromPropertyTree(const boost::property_tree::ptree & pt);
}

#endif // LOGMESSAGE_H

The problem here is that each of these files include the other which causes build errors. Unfortunately, I can't use the solution from the above-linked question (move #include "Logger.h" to Serializable.cpp) because Serializable is a template class, and therefore needs to be defined in the header file.

I'm at a loss for how to proceed. Any help is appreciated!

Edit: I also considered using a forward declarations of Logger and LogMessage inside serializable.h, but since I'm calling static methods in Logger and using LogMessage::Severity, that doesn't work.

As far as I know, you can't. Just don't write headers this way. e.g. merge them into one. — Cruz Jean, Apr 18 '19 at 18:28
Merging the headers would violate separation of concerns and doesn't scale nicely at all in a big project (which this is) — Kevin Rak, Apr 18 '19 at 18:28
Writing out (not in code) the intended relationships between the classes might help reveal unnecessary dependencies in the code. For instance: why does the `Severity` enumeration belong to `LogMessage` instead of to `Logger`? Why are `Logger` and `LogMessage` in the same header? Relevance: in the provided code (which I assume has been simplified), the only reason `Serializable` and `Logger` need the definition of `LogMessage` is to have access to the `Severity` enumeration, which seems a weak reason to need a class definition. — JaMiT, Apr 18 '19 at 19:12
@JaMit, you're right that the code has been simplified to show the problem. In reality, LogMessage and Logger are in separate headers. However, for the purpose of not providing too much unnecessary information, I omitted that. Similarly, Logger relies heavily on other features of LogMessage and does indeed need to include it. — Kevin Rak, Apr 19 '19 at 14:16
Actually, I think you might be right @JaMit. I might be able to move the necessary parts of LogMessage into a new file which both Logger and LogMessage include, without Logger including LogMessage. That would break the loop and resolve the issue. I'll try that tomorrow and report back with whether or not it worked. Thank you! — Kevin Rak, Apr 19 '19 at 20:55
@KevinRak it was not my intent to answer in the comments, but since my question might lead to a solution, I've gone ahead and collected some related thoughts into an official answer. (It got a bit long. Sorry about that.) — JaMiT, Apr 20 '19 at 06:38

score 5 · Accepted Answer · answered Apr 20 '19 at 06:37

Sometimes a circular dependency requires some analysis of the components that are involved. Figure out why the circle exists, then figure out why it does not need to exist. The analysis can happen at multiple levels. Here are the two levels I would start with.

(Since the code is obviously simplified from the real code, I'll avoid assuming it shows the extent of the true problem. Speaking of which, thanks for not inundating us with overwhelming detail! The code was sufficient to illustrate the problem in general. Any specific suggestions in my answers are similarly intended to illustrate points, not necessarily to be the final solution.)

On one level, you can look at the intent of the classes. Forget the code and focus on purpose. Does it make sense for class A to be unable to define itself without knowing what class B is? Keep in mind that this is stronger than knowing that class B exists (which would equate to a forward definition, header not required). If it doesn't make sense without looking at the code, then maybe you found something to work on. Admittedly, the use of templates complicates matters since the entire implementation needs to be in the header.

For example, a Serializable really should be able to define itself without knowing what will be done with the serialization (i.e. Logger). However, it is a template, and its implementation needs to be able to log an error. So... tricky.

Still, this is a place where one could look for solutions. One possibility might be to separate the error logging into a base piece that handles only strings (already serialized data), and a translation layer that can translate a LogMessage into a string for the base piece. An error during deserialization strongly suggests the lack of anything to serialize, so logging could go directly to the base piece. The dependency circle would break into the chains:

Serializable -> LoggerBase
Logger -> LoggerBase
Logger -> LogMessage -> Serializable -> LoggerBase

At another level, you can take a detailed look at the code, not worrying too much about purpose. You have header A including header B – why? What parts of A actually use something from B? Which parts of B are actually used? Draw up a diagram if you need to better visualize where this dependence lies. Then bring in some consideration of purpose. Are those parts defined in appropriate locations? Are there other possibilities?

For example, in the sample code, the reason Serializable needs LogMessage is to get access to the enumerate LogMessage::ERROR. This is not a strong reason for needing the entire LogMessage definition. Perhaps a wrapper like PostLogErrorSimple could remove the need to know the severity constant? Maybe the reality is more complicated than that, but the point is that some dependencies can be side-stepped by pushing the dependency into a source file. Sometimes the source file is for a different class.

Another example comes from the Logger class. This class requires LogMessage to get access to the LogMessage::Severity enumeration (i.e. the enumeration that has ERROR as one of its values). This is also not a strong reason for needing an entire class definition. Perhaps the enumeration should be defined elsewhere? As part of Logger perhaps? Or maybe not in a class definition at all? If this approach works, the dependency circle breaks into the chains:

Serializable -> Severity
Serializable -> Logger -> Severity // To get the PostLogMessageSimple function
Logger -> Severity

Ideally, once the enumeration is taken care of, the Logger header would be able get by with just a forward declaration of LogMessage instead of including the full header. (A forward declaration is enough to receive an object by reference. And presumably the full Logger definition would have some functions taking LogMessage parameters.)

It would be even better if there are some references (e.g. your blog). I am so addicted to your articles (SO answer). XD — javaLover, May 01 '19 at 11:07

score 0 · Answer 2 · answered Apr 20 '19 at 06:54

If you just have declarations in your class and define the methods out of line you should be able to make it work:

#pragma once
#ifndef SERIALIZABLE_H
#define SERIALIZABLE_H
#include <string>
#include <boost/property_tree/ptree.hpp>
#include <boost/property_tree/json_parser.hpp>
#include <boost/exception/diagnostic_information.hpp>
#include <boost/exception_ptr.hpp>

template<class T>
class Serializable {
public:
    static bool Deserialize(Serializable<T>* object, std::string serializedObject);
private:
    static boost::property_tree::ptree GetPropertyTreeFromJsonString(const std::string & jsonStr);
}

#include "Logger.h"

template < typename T >
inline bool Serializable<T>::Deserialize(Serializable<T>* object, std::string serializedObject) {
        try {
            return object->SetValuesFromPropertyTree(GetPropertyTreeFromJsonString(serialized));
        } catch (...) {
            std::string message = boost::current_exception_diagnostic_information();
            Logger::PostLogMessageSimple(LogMessage::ERROR, message);
            std::cerr << message << std::endl;
        }
    }

template < typename T >
inline boost::property_tree::ptree Serializable<T>::GetPropertyTreeFromJsonString(const std::string & jsonStr) {
        std::istringstream iss(jsonStr);
        boost::property_tree::ptree pt;
        boost::property_tree::read_json(iss, pt);
        return pt;
    }

#endif // SERIALIZABLE_H

This has the added bonus of making your class interface clearer.

Suppose a source file (`logger.cpp`?) begins by including `logger.h`. The first thing `logger.h` does is include `serializable.h`. This gives the definition of the `Serializable` class. Then there is an attempt to include `logger.h`, but that will fail because of the include guard. so we get down to the implementations. Which try to call `Logger::PostLogMessageSimple` but that has not been declared yet? I think there's still a problem. Unless you want to prevent anyone but `serializable.h` from including `logger.h`? — JaMiT, Apr 20 '19 at 13:44
@JaMiT, putting the include of Serializable before the include guard in logger would solve that i think — Alan Birtles, Apr 21 '19 at 06:17
When I try this approach I get a compiler error on the line with Logger::PostLogMessageSimple: 'Logger' has not been declared. Seems related to this: https://stackoverflow.com/a/16628994/4875896 — Kevin Rak, Apr 22 '19 at 16:05
Putting the include of Serializable before the include guard in LogMessage causes a lot of other issues with circular dependencies. — Kevin Rak, Apr 22 '19 at 16:17
@AlanBirtles Sounds plausible, but then the change to the logger header should also be in this answer. — JaMiT, Apr 23 '19 at 02:54

score 0 · Answer 3 · answered Mar 14 '22 at 15:47

Consider N headers that each define a class. Let each of the N headers contain a template or inline function, that therefore needs to be defined in the header, that uses the declaration of all other N-1 classes.

In the example below I use N=4, struct instead of class and an inline member function instead of a template, to actually use the other structs. Of course, each struct definition also use (need) the forward declarations, but I left that out because it isn't relevant for the pattern.

A.h:

#ifndef A_H
#define A_H

// Forward declare everything.
struct A;
struct B;
struct C;
struct D;

struct A {
  void use();
};

#endif // A_H

#ifndef B_H
#include "B.h"
#endif
#ifndef C_H
#include "C.h"
#endif
#ifndef D_H
#include "D.h"
#endif

#ifndef A_defs_H
#define A_defs_H

inline void A::use()
{
  // Use everything.
  B x; C y; D z;
}

#endif // A_defs_H

B.h:

#ifndef B_H
#define B_H

// Forward declare everything.
struct A;
struct B;
struct C;
struct D;

struct B {
  void use();
};

#endif // B_H

#ifndef A_H
#include "A.h"
#endif
#ifndef C_H
#include "C.h"
#endif
#ifndef D_H
#include "D.h"
#endif

#ifndef B_defs_H
#define B_defs_H

inline void B::use()
{
  // Use everything.
  A x; C y; D z;
}

#endif // B_defs_H

C.h:

#ifndef C_H
#define C_H

// Forward declare everything.
struct A;
struct B;
struct C;
struct D;

struct C {
  void use();
};

#endif // C_H

#ifndef A_H
#include "A.h"
#endif
#ifndef B_H
#include "B.h"
#endif
#ifndef D_H
#include "D.h"
#endif

#ifndef C_defs_H
#define C_defs_H

inline void C::use()
{
  // Use everything.
  A x; B y; D z;
}

#endif // C_defs_H

D.h:

#ifndef D_H
#define D_H

// Forward declare everything.
struct A;
struct B;
struct C;
struct D;

struct D {
  void use();
};

#endif // D_H

#ifndef A_H
#include "A.h"
#endif
#ifndef B_H
#include "B.h"
#endif
#ifndef C_H
#include "C.h"
#endif

#ifndef D_defs_H
#define D_defs_H

inline void D::use()
{
  // Use everything.
  A x; B y; C z;
}

#endif // D_defs_H

The guards around each #include are necessary to avoid infinite include depths. Note with just two headers (N = 2) you can also put the includes inside the previous block. For example:

A.h:

#ifndef A_H
#define A_H

// Forward declare everything.
struct A;
struct B;

struct A {
  void use();
};

#include "B.h"
#endif // A_H

#ifndef A_defs_H
#define A_defs_H

inline void A::use()
{
  // Use everything.
  B x;
}

#endif // A_defs_H

and likewise for B.h works fine with N = 2.

How can I handle circular #include calls in a template class header?

3 Answers3