Inheritance and LSP

Apologies in advance for a long-winded question. Feedback especially appreciated here . . .

In my work, we do a lot of things with date ranges (date periods, if you will). We need to take all sorts of measurements, compare overlap between two date periods, etc. I have designed an Interface, a base class, and several derived classes which serve my needs well to date:

• IDatePeriod
• DatePeriod
• CalendarMonth
• CalendarWeek
• FiscalYear

Stripped to its essentials, the DatePeriod superclass is as follows (omits all the fascinating features which are the basis for why we need this set of classes . . .):

(Java pseudocode):

class datePeriod implements IDatePeriod

protected Calendar periodStartDate
protected Calendar periodEndDate

    public DatePeriod(Calendar startDate, Calendar endDate) throws DatePeriodPrecedenceException
    {
        periodStartDate = startDate
        . . . 
        // Code to ensure that the endDate cannot be set to a date which 
        // precedes the start date (throws exception)
        . . . 
        periodEndDate = endDate
    {

    public void setStartDate(Calendar startDate)
    {
        periodStartDate = startDate
        . . . 
        // Code to ensure that the current endDate does not 
        // precede the new start date (it resets the end date
        // if this is the case)
        . . . 
    {


    public void setEndDate(Calendar endDate) throws datePeriodPrecedenceException
    {
        periodEndDate = EndDate
        . . . 
        // Code to ensure that the new endDate does not 
        // precede the current start date (throws exception)
        . . . 
    {


// a bunch of other specialty methods used to manipulate and compare instances of DateTime

}

The base class contains a bunch of rather specialized methods and properties for manipulating the date period class. The derived classes change only the manner in which the start and end points of the period in question are set. For example, it makes sense to me that a CalendarMonth object indeed "is-a" DatePeriod. However, for obvious reasons, a calendar month is of fixed duration, and has specific start and end dates. In fact, while the constructor for the CalendarMonth class matches that of the superclass (in that it has a startDate and endDate parameter), this is in fact an overload of a simplified constructor, which requires only a single Calendar object.

In the case of CalendarMonth, providing any date will result in a CalendarMonth instance which begins on the first day of the month in question, and ends on the last day of that same month.

public class CalendarMonth extends DatePeriod

    public CalendarMonth(Calendar dateInMonth)
    {
        // call to method which initializes the object with a periodStartDate
        // on the first day of the month represented by the dateInMonth param,
        // and a periodEndDate on the last day of the same month.
    }

    // For compatibility with client code which might use the signature
    // defined on the super class:
    public CalendarMonth(Calendar startDate, Calendar endDate)
    {
        this(startDate)
        // The end date param is ignored. 
    }

    public void setStartDate(Calendar startDate)
    {
        periodStartDate = startDate
        . . . 
    // call to method which resets the periodStartDate
    // to the first day of the month represented by the startDate param,
    // and the periodEndDate to the last day of the same month.
        . . . 
    {


    public void setEndDate(Calendar endDate) throws datePeriodPrecedenceException
    {
        // This stub is here for compatibility with the superClass, but
        // contains either no code, or throws an exception (not sure which is best).
    {
}

Apologies for the long preamble. Given the situation above, it would seem this class structure violates the Liskov substitution principle. While one CAN use an instance of CalendarMonth in any case in which one might use the more general DatePeriod class, the output behavior of key methods will be different. In other words, one must be aware that one is using an instance of CalendarMonth in a given situation.

While CalendarMonth (or CalendarWeek, etc.) adhere to the contract established through the base class' use of IDatePeriod, results might become horribly skewed in a situation in which the CalendarMonth was used and the behavior of plai开发者_高级运维n old DatePeriod was expected . . . (Note that ALL of the other funky methods defined on the base class work properly - it is only the setting of start and end dates which differs in the CalendarMonth implementation).

Is there a better way to structure this such that proper adherence to LSP might be maintained, without compromising usability and/or duplicating code?

This seems similar to the usual discussion about Squares and Rectangles. Although a square is-a rectangle, it's not useful for Square to inherit from a Rectangle, because it cannot satisfy the expected behavior of a Rectangle.

Your DatePeriod has a setStartDate() and setEndDate() method. With a DatePeriod, you would expect that the two could be called in any order, would not affect each other, and maybe that their values would precisely specify a start and end date. But with a CalendarMonth instance, that's not true.

Maybe, instead of having CalendarMonth extend DatePeriod, the two could both extend a common abstract class, that contains only methods compatible with both.

By the way, based on the thoughtfulness of your question, I'm guessing you've already thought to look for existing date libraries. Just in case you haven't, be sure to take a look at the Joda time library, which includes classes for mutable and immutable periods. If an existing library solves your problem, you could concentrate on your own software, and let someone else pay the cost of designing, developing and maintaining the time library.

Edit: Noticed I had referred to your CalendarMonth class as Calendar. Fixed for clarity.

I think the modeling problem is that your CalendarMonth type isn't really a different kind of period. Rather, it's a constructor or, if you prefer, factory function for creating such periods.

I'd eliminate the CalendarMonth class and create a utility class called something like Periods, with a private constructor and various public static methods that return various IDatePeriod instances.

With that, one could write

final IDatePeriod period = Periods.wholeMonthBounding(Calendar day);

and the documentation for the wholeMonthBounding() function would explain what the caller can expect of the returned IDatePeriod instance. Bikeshedding, an alternate name for this function could be wholeMonthContaining().

Consider what you intend to do with your "periods". If the goal is do "containment testing", as in "Does this moment sit within some period?", then you might like to acknowledge infinite and half-bounded periods.

That suggests that you'd define some containment predicate type, such as

interface PeriodPredicate
{
  boolean containsMoment(Calendar day);
}

Then the aforementioned Periods class—perhaps bettern named PeriodPredicates with this elaboration—could expose more functions like

// First, some absolute periods:
PeriodPredicate allTime(); // always returns true
PeriodPredicate everythingBefore(Calendar end);
PeriodPredicate everythingAfter(Calendar start);
enum Boundaries
{
  START_INCLUSIVE_END_INCLUSIVE,
  START_INCLUSIVE_END_EXCLUSIVE,
  START_EXCLUSIVE_END_INCLUSIVE,
  START_EXCLUSIVE_END_EXCLUSIVE
}
PeriodPredicate durationAfter(Calendar start, long duration, TimeUnit unit,
                              Boundaries boundaries);
PeriodPredicate durationBefore(Calendar end, long duration, TimeUnit unit
                               Boundaries boundaries);

// Consider relative periods too:
PeriodPredicate inThePast();   // exclusive with now
PeriodPredicate inTheFuture(); // exclusive with now
PeriodPredicate withinLastDuration(long duration, TimeUnit unit); // inclusive from now
PeriodPredicate withinNextDuration(long duration, TimeUnit unit); // inclusive from now
PeriodPredicate withinRecentDuration(long pastOffset, TimeUnit offsetUnit,
                                     long duration, TimeUnit unit,
                                     Boundaries boundaries);
PeriodPredicate withinFutureDuration(long futureOffset, TimeUnit offsetUnit,
                                     long duration, TimeUnit unit,
                                     Boundaries boundaries);

That should be enough of a push. Let me know if you need any clarification.

Often times adhering to the LSP is a matter of being meticulous about documenting what the base class or interface does.

For instance, in Java Collection has a method named add(E). It could have this documentation:

Adds the specified element to this collection.

But if it did, then it would be very difficult for a Set, which maintains a no-duplicates invariant, to not violate the LSP. So instead, add(E) is documented like this:

Ensures that this collection contains the specified element (optional operation).

Now no client can use a Collection and expect that the element will always be added even if it already existed in the collection.

I haven't looked too in-depth at your example, but it strikes me you might be able to be as careful. What if your in your date period interface, setStartDate() was documented like this:

Ensures that the start date is the date specified.

Without specifying anything further? Or even,

Ensures that the start date is the date specified, optionally changing the end date to maintain any specific invariants of the subclass.

setEndDate() could be implemented and be similarily documented. How then would a concrete implementation break the LSP?

Note It is also worth mentioning that it's a lot easier to satisfy LSP if you make your class immutable.

This certainly does violate LSP, in exactly same way as the classic Ellipse and Circle example.

If you want CalendarMonth to extend DatePeriod, you should make DatePeriod immutable.

You can then either change all the mutating methods to ones that return a new DatePeriod and keep everything nicely immutable, or make alternate mutable subclass that doesn't try to deal with years, months, weeks and such.