Large class, small class: Extracting Small Blocks of Functionality

The general move in object-oriented design has been towards the idea of one class with one responsibility.

This obviously needs some interpretation.  If a class is relatively close to main() in the calling hierarchy, it may very well legitimately have a lot of things it deals with, in order to provide a simple interface to its calling context.  But the general idea is that when that is the case, most of the logic should be encapsulated in next-level-down objects, and its single responsibility is described in terms of managing them.

I am not here promoting the notion of old-style "manager" classes, which tended to become blob antipatterns at the drop of a hat.  But consider the "Vespers" class.  It represents one thing, but that thing is made up of multiple parts -- antiphons, chapter, sets of psalms, etc. The Vespers class has eight members, but (aside from its constructor) only one public function, to manage the assembly of those parts into a formatted output stream. It's immutable.

The corresponding VespersElement has a single job (representing an XML Vespers description on disk).  It has more than one accessor but all of the functions are ways for the rest of the application to use that information as a coherent whole.  So one job == three functions, all accessors, in an immutable class.

Both classes also show another bias, or rather two related biases: first, classes should, whenever possible, be immutable. (Doubled, redoubled, and in spades if in a multi-threaded environment.) Secondly, complexity tends to be allowed in a constructor which you would not normally see in any other functions. (If you see complex non-constructor functions one's obvious first question is "can parts of this be encapsulated?"  In a constructor the complexity is usually a part of setting up the encapsulation in the first place, though even there reducing immediate complexity is a good thing.)

The next implication is that any grouping of activity around one or two data members starts to look like a blurring of responsibility -- because frequently that logic could be encapsulated, even if the result was a very small class.

(Not as small as possible.  It's perfectly reasonable to have classes of a very few lines with one member if you are introducing a class like MessageID as a wrapper around a string which is to be passed around as a parameter, i.e.

class MessageID {

public:

    explicit MessageID(const std::string& inVal): m_body(inVal) { }

    const std::string& getID() const { return m_body; }

private:

    std::string m_body;

};

This would then be used in parameter lists rather than a bare string.  But this belongs not to the broad category of factoring out functionality but of improving type-safety, which is an entirely different topic.)

So: I have a class dealing with determining the name and season of a day in the proper of time.  It "memorizes" a few Sundays (represented as std::chrono::year_month_day) because they are special in one way or another (key pivots in the year, or major feasts that are not in Holy Week, are in the Easter cycle, and are not Sundays).

Most of the Sundays are used fairly straightforwardly; once CorpusChristi has been calculated, it is simply used to compare a date against in an accessor.  But Advent Sunday is a bit different.

It has a more complex constructor (essentially, begin from Christmas, get its weekday, figure out the resulting date of the beginning of Advent), and it's used as a sentinel in several places because it marks a sharp break in seasons (though only one of those uses is outside the class constructor).  It's not a huge amount of logic, but it does form a cluster.  The one catch is that it needs two-stage initialization: calculating the date of Christmas occurs a little way into the parent class constructor. (This can be managed by having a default initializer and an initializer with a value and then using move assignment: there is no need to create a modifying set() method.)

Encapsulating this gives us the following:

.h File:

class AdventSunday

{

public:

  AdventSunday() {}

  AdventSunday(const std::chrono::year_month_day inChristmas);

  std::chrono::year_month_day getSundayNextBeforeAdvent() const { return { m_absoluteDay - std::chrono::days{ 7 } }; }

  std::chrono::year_month_day getAdventII() const { return { m_absoluteDay - std::chrono::days{ 7 } }; }

  std::chrono::year_month_day getAdventI() const { return m_day; }

  bool isBeforeAdvent(std::chrono::sys_days inDate) const { return inDate < m_absoluteDay; }

private:

  std::chrono::year_month_day m_day;

  std::chrono::sys_days m_absoluteDay;

};

.cpp file:

namespace

{

std::chrono::year_month_day

ChristmasToAdvent(std::chrono::year_month_day inVal)

{

  switch (std::chrono::weekday{ std::chrono::sys_days{ inVal } }.c_encoding())

    {

    case 5:

      return std::chrono::year_month_day{ inVal.year(),

                                          std::chrono::month{ 11 }, 29d };

      break;

    case 1:

      return std::chrono::year_month_day{ inVal.year(),

                                          std::chrono::month{ 12 }, 3d };

      break;

    case 6:

      return std::chrono::year_month_day{ inVal.year(),

                                          std::chrono::month{ 11 }, 28d };

      break;

    case 0:

      return std::chrono::year_month_day{ inVal.year(),

                                          std::chrono::month{ 11 }, 27d };

      break;

    case 4:

      return std::chrono::year_month_day{ inVal.year(),

                                          std::chrono::month{ 11 }, 30d };

      break;

    case 2:

      return std::chrono::year_month_day{ inVal.year(),

                                          std::chrono::month{ 12 }, 2d };

      break;

    case 3:

      return std::chrono::year_month_day{ inVal.year(),

                                          std::chrono::month{ 12 }, 1d };

      break;

    }

  return {};

}

}


namespace BreviaryLib

{

AdventSunday::AdventSunday(const std::chrono::year_month_day inChristmas):

    m_day{ ChristmasToAdvent(inChristmas) }, m_absoluteDay{

      std::chrono::sys_days{ m_day }

    }

{ }

There's one additional tweak.  The normal context will have the current year as the year being handled, so if we tweak the constructor in which it's called, we can just let the default constructor set the date for this year, and not have to reset.  This also avoids ever having an instance with an undefined value in its date member. So we have:

AdventSunday::AdventSunday()

{

  const std::chrono::time_point now{ std::chrono::system_clock::now() };

  const std::chrono::year_month_day ymd{ std::chrono::floor<std::chrono::days>(

      now) };

  m_day = ChristmasToAdvent(

      std::chrono::year_month_day(ymd.year(), std::chrono::month{ 12 }, 25d));

  m_absoluteDay = std::chrono::sys_days{ m_day };

}

rather than the no-op implementation in the header above.

That's about 95 lines of code in total, including blank lines.  No operations are repeated, i.e. this doesn't do any replacing of two or more locations with one function.

But is it a worthwhile change? Yes, because the places where it is called increase in clarity. The main benefit is moving that switch logic out of the middle of the DateCalculator constructor, and in the slight increase in legibility when named calls are replaced for bare operations which they now replace.  It's also a small, easy class to unit test: prior to this the logic was embedded in other construction logic which made it more difficult to target.

Comments

Post a Comment

Popular posts from this blog

LT Project: Author

The LTLibrary Author class is designed to do two things: provide access to the author's last  name, and to provide a lexicographic sort key based on the author's full name.  The expected format of the author name is "Smith, John", or "Aristotle".  For sorting, punctuation and spaces are removed and the string is converted to lower case. namespace LtLibrary { class Author { public:   void set(const std::string &inName);   void set(const std::string &inName, const std::string &inCompName);   auto empty() const { return m_name.empty(); }   auto   compare(const Author &inOther) const   {     return m_compName.compare(inOther.m_compName);   }   auto containsName(std::string_view inName) const { return m_name.contains(inName); }   std::string_view getLastName() const; private:   std::string m_name;   std::string m_compName; }; } inline bool operator==(const LtLibrary::Author& inFirst, const LtL...
Read more

Boundaries

Imagine that you have a well-designed application, possibly using dependency injection. It will almost always have a very thin layer visible in main() - capturing command-line parameters and environment variables, mainly[1], and then a call to an Application class which manages the rest of the initial logic - setting up resources such as logging, doing substantive validation of parameters, and creating and running the objects on the next level down which actually carry out the work of the application. This is simply following the SRP at a high level.
Read more

Decent First Drafts

Let's say that you are implementing a Composite pattern for an interface with several interface functions. For functions which return void, it's straightforward to anticipate that the implementation will be a simple for_each. For functions which return a value, something has to be done to convert multiple return values into one. There are several obvious patterns: - For functions like size() you call accumulate/fold_left and return an accumulated value.  This can't be extended arbitrarily to functions returning a numeric type: you might want, for example, to return a maximum value. - For functions which return a boolean status, you return the results of applying std::none_of to a test for a false return, std::all_of to a test for a true return. - For functions generating a string representation you return a string concatenating outputs with interpolated delimiters and possibly a beginning and ending. The same applies as a special case of output functions which return void b...
Read more