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.
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.)