LT Project: Record Sets
Individual library book records are stored in sets, and the applications typically interact with the sets rather than with individual records. If my compiler had better support for modules, the interface in the domain would be part of the published part of the module, and almost everything we've looked at until now would be unpublished. (ILibraryBookRecord also might need to be published because the formatting mechanisms might need to be extended for specialized cases, though to date the only thing that needs to be available is the existence, not the contents of ILibraryBookRecord. This would allow the record interface not to be published if the core IFormatter implementations were in the same module.)
It's not a long interface:
class ILibraryRecordSet
{
public:
virtual ~ILibraryRecordSet();
virtual void print(const IFormatter &inFormatter) const = 0;
virtual void addToCollection(ILibraryBookRecord *inRec) = 0;
virtual std::string_view getTitle() const = 0;
virtual std::string_view getAuthorLastName !() const = 0;
virtual std::size_t size() const = 0;
};
It's not immutable because the building mechanism, iterating over a collection of candidate records, means having to build it up incrementally.
getTitle() and getAuthorLastName() are used in some application contexts when it is expected that there is one item in the collection, as a mechanism for avoiding breaking encapsulation.
The simplest implementation is, as almost always, a NullObject implementation, in this case "empty":
namespace LtLibrary
{
class EmptyLibraryRecordSet : public ILibraryRecordSet
{
public:
~EmptyLibraryRecordSet() override;
void print(const IFormatter &inFormatter) const override;
void addToCollection(ILibraryBookRecord *inRec) override
{ }
std::string_view getTitle() const override;
std::string_view getAuthorLastName() const override;
std::size_t size() const override
{
return 0;
}
};
The print() function is *not* a no-op, because we want to display when a search has failed:
void EmptyLibraryRecordSet::print(const IFormatter &inFormatter) const
{
static NullLibraryBookRecord rec;
inFormatter.formatRecord(rec);
}
As you might expect, the two get() functions return empty strings:
std::string_view EmptyLibraryRecordSet::getTitle() const
{
return ""sv;
}
std::string_view EmptyLibraryRecordSet::getAuthorLastName() const
{
return ""sv;
}
The next most complex implementation is a one-element set. This gets fairly heavy use in some contexts (retrieving a record based on ID) so it makes sense to optimize it rather than just making it a case of the general storage (which can still have a single member if a more general search which might have retrieved more items).
class SingleLibraryRecordSet : public ILibraryRecordSet,
public boost::noncopyable
{
public:
SingleLibraryRecordSet(const BaseLibraryBookRecord &inRec,
const IRecordSetPopulator &inPopulator,
std::shared_ptr<Log::ILogger> inLogger);
~SingleLibraryRecordSet() override;
void print(const IFormatter &inFormatter) const override;
void addToCollection(ILibraryBookRecord *inRec) override
{
m_record.reset(inRec);
}
std::string_view getTitle() const override
{
return m_record->getTitle();
}
std::string_view getAuthorLastName() const override
{
return m_record->getAuthorLastName();
}
std::size_t size() const override
{
return 1;
}
private:
std::unique_ptr<ILibraryBookRecord> m_record;
};
The constructor is error-handling-heavy:
SingleLibraryRecordSet::SingleLibraryRecordSet(
const BaseLibraryBookRecord &inRec, const IRecordSetPopulator &inPopulator,
std::shared_ptr<Log::ILogger> inLogger)
{
try
{
inPopulator.filterRecord(inRec)->addToCollection(*this);
}
catch (const std::system_error &e)
{
std::ostringstream str;
str << " On adding to set from filter: " << e.what() << ": " << inRec;
inLogger->log(Log::Severity::Fatal, str.str(), e.code());
throw;
}
catch (const std::exception &e)
{
std::ostringstream str;
str << " On adding to set from filter: " << e.what() << ": " << inRec;
inLogger->log(Log::Severity::Fatal, str.str());
throw;
}
if (m_record.get() == nullptr)
{
inLogger->log(Log::Severity::Fatal,
"Retrieved item does not match id in populator");
throw std::runtime_error(
"Retrieved item does not match id in populator");
}
}
But given that it is a constructor and this is the only place where errors can occur to be logged, we won't try to extract the logging to a separate class. (It's still a simpler constructor than the general case.)
We will look, later, at the IRecordSetPopulator class and its implementations. For now, I'll just provide the interface for context:
class IRecordSetPopulator
{
public:
virtual ~IRecordSetPopulator ();
virtual ILibraryBookRecord *
createRecord (const std::string &inString) const = 0;
virtual ILibraryBookRecord *
filterRecord (const BaseLibraryBookRecord &inRec) const = 0;
virtual void updateStats (LtStats &outStats,
const BaseLibraryBookRecord &inRec) const = 0;
};
The other three calls are where the real payoff occurs, as there's a simple passthrough for all three calls, two of which are inlined (the third is not to provide insulation for the IFormatter interface):
void SingleLibraryRecordSet::print(const IFormatter &inFormatter) const
{
inFormatter.formatRecord(*m_record);
}
The full LibraryRecordSet is, naturally, the most elaborate. This can be seen simply in the declaration:
class LibraryRecordSet : public ILibraryRecordSet, public boost::noncopyable
{
class ILogger
{
public:
virtual ~ILogger();
virtual void logSystemError(const std::string &inContext,
const std::system_error &e,
const std::string &inData) const = 0;
virtual void logException(const std::string &inContext,
const std::exception &e,
const std::string &inData) const = 0;
virtual void
logSystemError(const std::string &inContext,
const std::system_error &e,
const BaseLibraryBookRecord &inRec) const = 0;
virtual void
logException(const std::string &inContext, const std::exception &e,
const BaseLibraryBookRecord &inRec) const = 0;
};
class Logger : public ILogger
{
public:
Logger(std::shared_ptr<Log::ILogger> inLogger): m_logger(inLogger) { }
~Logger() override;
void logSystemError(const std::string &inContext,
const std::system_error &e,
const std::string &inData) const override;
void logException(const std::string &inContext,
const std::exception &e,
const std::string &inData) const override;
void
logSystemError(const std::string &inContext,
const std::system_error &e,
const BaseLibraryBookRecord &inRec) const override;
void
logException(const std::string &inContext, const std::exception &e,
const BaseLibraryBookRecord &inRec) const override;
private:
std::shared_ptr<Log::ILogger> m_logger;
static void ErrorToString(std::ostream &outStream,
const std::string &inContext,
const std::exception &e);
};
public:
LibraryRecordSet(std::span<const std::string> inVec,
const IRecordSetPopulator &inPopulator,
const bool isBibliographic,
std::shared_ptr<Log::ILogger> inLogger);
LibraryRecordSet(std::span<const BaseLibraryBookRecord> inVec,
const IRecordSetPopulator &inPopulator,
const bool isBibliographic,
std::shared_ptr<Log::ILogger> inLogger);
void print(const IFormatter &inFormatter) const override;
void addToCollection(ILibraryBookRecord *inRec) override;
std::string_view getTitle() const override;
std::string_view getAuthorLastName() const override;
std::size_t size() const override
{
return m_records.size();
}
private:
void add(const std::string &inStr, const IRecordSetPopulator &inPopulator);
void filterAdd(const BaseLibraryBookRecord &inRec,
const IRecordSetPopulator &inPopulator);
void sortRecords(const bool inIsBibliographic);
boost::ptr_vector<ILibraryBookRecord> m_records;
std::unique_ptr<ILogger> m_logger;
};
There is enough error logging to warrant a logger class to capture it. An interface is provided (plus a couple of specialized constructors, omitted above) to allow mocked loggers to be passed in in a unit test context.
The member records are stored in a boost::ptr_vector, which manages the memory and simplifies the iteration code. This is one of the areas where there is an argument for using bare new() rather than make_unique() in creating the elements to begin with, as the clone() methods do.
The two functions which are meaningful only when there is a single member reflect that:
std::string_view LibraryRecordSet::getTitle() const
{
if (m_records.size() != 1)
return ""sv;
return m_records.back().getTitle();
}
std::string_view LibraryRecordSet::getAuthorLastName() const
{
if (m_records.size() != 1)
return ""sv;
return m_records.back().getAuthorLastName();
}
The private methods add() and filterAdd() assist in the construction of the collection.
void LibraryRecordSet::add(const std::string &inStr,
const IRecordSetPopulator &inPopulator)
{
try
{
inPopulator.createRecord(inStr)->addToCollection(*this);
}
catch (const std::system_error &e)
{
m_logger->logSystemError("add()", e, inStr);
throw;
}
catch (const std::exception &e)
{
m_logger->logException("add()", e, inStr);
throw;
}
}
void LibraryRecordSet::filterAdd(const BaseLibraryBookRecord &inRec,
const IRecordSetPopulator &inPopulator)
{
try
{
inPopulator.filterRecord(inRec)->addToCollection(*this);
}
catch (const std::system_error &e)
{
m_logger->logSystemError("filterAdd()", e, inRec);
throw;
}
catch (const std::exception &e)
{
m_logger->logException("filterAdd()", e, inRec);
throw;
}
}
(The public method
void LibraryRecordSet::addToCollection(ILibraryBookRecord *inRec)
{
m_records.push_back(inRec);
}
is actually called in a double-dispatch mechanism under the record's addToCollection() call. This means that although in principle the addToCollection() call prevents the interface from being immutable, in practice it is called only during the construction phase, here.)
sortRecords() is also called in the construction context. It ensures that an appropriate sort order is set and then calls the member function sort() on the ptr_vector. (std::sort does not work with boost pointer containers and is replaced with a member function.)
void LibraryRecordSet::sortRecords(const bool inIsBibliographic)
{
try
{
if (inIsBibliographic)
LibraryBookRecord::SetBibliographicSort();
else
LibraryBookRecord::SetTitleSort();
m_records.sort();
}
catch (const std::system_error &e)
{
m_logger->logSystemError("sortRecords()", e, "");
throw;
}
catch (const std::exception &e)
{
m_logger->logException("sortRecords()", e, "");
throw;
}
}
Here is how the constructors put it together:
LibraryRecordSet::LibraryRecordSet(std::span<const std::string> inVec,
const IRecordSetPopulator &inPopulator,
const bool isBibliographic,
std::shared_ptr<Log::ILogger> inLogger)
: m_logger(std::make_unique<Logger>(inLogger))
{
std::ranges::for_each(
inVec, [&](const auto &inVal) { add(inVal, inPopulator); });
sortRecords(isBibliographic);
}
LibraryRecordSet::LibraryRecordSet(
std::span<const BaseLibraryBookRecord> inVec,
const IRecordSetPopulator &inPopulator, const bool isBibliographic,
std::shared_ptr<Log::ILogger> inLogger)
: m_logger(std::make_unique<Logger>(inLogger))
{
std::ranges::for_each(
inVec, [&](const auto &inVal) { filterAdd(inVal, inPopulator); });
sortRecords(isBibliographic);
}
Because sortRecords() and the addition functions already capture and log errors there is no need for an additional try/catch block in the constructors themselves.
A tiny note on C++03 versus C++20:
The initialization loop in the two constructors uses std::ranges::for_each combined with lambdas
In C++03 these would be expressed as one of (noting that there is also no span, so the argument they would handle would be explicitly a vector):
for (std::vector<std::string>::const_iterator iter = inVec.begin();
iter != inVec.end();
filterAdd((*iter), inPopulator));
or:
class Adder {
public:
Adder(LibraryRecordSet& inParent,
const IRecordSetPopulator &inPopulator):
m_parent(inParent),
m_populator(inPopulator)
{ }
void operator()(const std::string& inVal) { m_parent.filterAdd(inVal, m_populator); }
private:
LibraryRecordSet& m_parent;
const IRecordSetPopulator &m_populator;
};
Adder a(*this, inPopulator);
std::for_each(inVec.begin(), inVec.end(), a);
It's no wonder that a lot of developers preferred to stick with old-fashioned for loops... In C++20, both are now less simple than using the STL algorithm.
(The for_each model is actually still better than the for loop, because the constructors using possible mock loggers, not shown here, reuse the iteration logic, so the Adder class would actually be moved out of the function and reused.)
An Excursus on Logging
The logger I've provided here captures some fairly simple logic with a fair number of lines of overhead:
void LibraryRecordSet::Logger::logSystemError(const std::string &inContext,
const std::system_error &e,
const std::string &inData) const
{
std::ostringstream str;
ErrorToString(str, inContext, e);
str << inData;
m_logger->log(Log::Severity::Fatal, str.str(), e.code());
}
void LibraryRecordSet::Logger::logException(const std::string &inContext,
const std::exception &e,
const std::string &inData) const
{
std::ostringstream str;
ErrorToString(str, inContext, e);
str << inData;
m_logger->log(Log::Severity::Fatal, str.str());
}
void LibraryRecordSet::Logger::logSystemError(
const std::string &inContext, const std::system_error &e,
const BaseLibraryBookRecord &inRec) const
{
std::ostringstream str;
ErrorToString(str, inContext, e);
inRec.print(str);
m_logger->log(Log::Severity::Fatal, str.str(), e.code());
}
void LibraryRecordSet::Logger::logException(
const std::string &inContext, const std::exception &e,
const BaseLibraryBookRecord &inRec) const
{
std::ostringstream str;
ErrorToString(str, inContext, e);
inRec.print(str);
m_logger->log(Log::Severity::Fatal, str.str());
}
void LibraryRecordSet::Logger::ErrorToString(std::ostream &outStream,
const std::string &inContext,
const std::exception &e)
{
outStream << "LibraryRecordSet::" << inContext << e.what() << ":";
}
This reflects my general use whenever a class has more than a minimal level of detailed logging.
Why do this?
I have several reasons.
First, and most importantly, the way that we manage complexity in programming generally is in two ways: we hide it with an extra level of indirection (that's simply standard "small classes" but also very, very standard structured programming as it's been since Djikstra) or we move it around (also small classes). Moving it around means, typically, making logical flows clear for the maintainer by reducing clutter. Moving it around is a big part of what the SRP is all about.
If you can, you move complexity to a constructor. You move failure conditions to a constructor, on fail-fast principles. You set data up so that later operations can be efficient. It's easier to live with it in one place. You purchase general algorithmic clarity by doing more stuff up front.
If behaviour can vary, and the variation is stable for the lifetime of an object, you move complexity out by using strategies rather than if/else tests on boolean flags. This not only cleans up the code (if/else gets moved the the constructor, but also the variant logic gets moved out of the class proper entirely) but is frequently a little faster (virtual call versus branching, but don't count on that without profiling).
Error handling in general is a problem: at a guess about 80% of production code in the real world is error handling. The standard "Hello World" program can be clear because nobody normally checks for whether the console exists (although sometimes it doesn't). As soon as you try to write Hello World to file you should be checking for whether the file can be opened, generating an error message if it can't (and distinguish between "no permissions", "location does not exist", "no space left on device" and "No inodes left on device"), and also checking the success of the write call.
Logging is not only a big part of error handling but in a typical production system you provide ongoing logging to allow support to diagnose the causes of unexpected (and frequently not erroneous) behaviour. That sort of logging can't be confined to a constructor.
So logging is clutter. The four lines in my logging functions above aren't much as far as complexity goes, but they add up. Having a logging class to group the logging functions together not only follows the SRP, not only does some (minimal) encapsulation of logic, but it means that the overhead of handling exceptions, which is already messy, is reduced.
Secondly, when I'm unit testing I frequently want to capture error conditions. Tracking and verifying the handling of error conditions means tracking the logging of values. It's frequently very helpful to be able to mock the logging so that (1) it can be captured and also so that (2) the tests can be simpler. (If you just pass in a mocked general logger, your tests are against the (somewhat wordy) output, A mocked call at the logical level is a much clearer test.)
Thirdly, I tend to find that the discipline of writing a logger leads to insights that allow for further cleanup of the code. This is simply general experience, and nothing I can quantify.
Comments
Post a Comment