Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: needs more work

...

  • Understand what the superclass does and watch out for mutating functionality
  • Make sure that new methods that are added to the superclass are overridden appropriately if there is some division of logic
  • Never modularize in absurd ways as is apparent in the noncompliant code example

Noncompliant Code Example

This noncompliant example overrides the methods after() and compareTo() of the class java.util.Calendar. The Calendar.after() method returns a boolean value depending on whether the Calendar represents a time after the time represented by the specified Object parameter. The programmer wishes to extend this functionality and return true even when the two objects are equal. If it is the first day of week it must return true, else run a regular comparison. Note that compareTo() is also overridden in this example, to provide a "comparisons by day" option to clients. For example, comparing today's day with the first day of week (which differs from country to country) to check whether it is a weekday.

Typically, errors manifest when assumptions are made about the implementation specific details of the superclass. Here, the two objects are compared for equality in the overriding after() method and subsequently, the superclass's after() method is explicitly called to take over. The issue is that the superclass Calendar's after() method in turn internally uses the compareTo() method. The superclass's after() method erroneously invokes the subclass's version of compareTo() due to polymorphism. Since the subclass is unaware of the superclass's implementation of after(), it does not expect any of its own overriding methods to get invoked.

Code Block
bgColor#FFCCCC

class CalendarSubclass extends Calendar {
  @Override public boolean after(Object when) {
    if(when instanceof Calendar && compareTo((Calendar)when) == 0) // correctly calls CalendarSubclass.compareTo()
      return true;
    return super.after(when); // incorrectly calls CalendarSubclass.compareTo()
  }
	
  @Override public int compareTo(Calendar anotherCalendar) {
    return compareTo(anotherCalendar.getFirstDayOfWeek(),anotherCalendar);
  }

  private int compareTo(int firstDayOfWeek, Calendar c) {
    int thisTime = c.get(Calendar.DAY_OF_WEEK);
    return (thisTime > firstDayOfWeek) ? 1 : (thisTime == firstDayOfWeek) ? 0 : -1;
  }

  public static void main(String[] args) {
    CalendarSubclass cs = new CalendarSubclass();
    cs.set(Calendar.DAY_OF_WEEK, 3); // set the day to the third day (Tuesday in US)
    System.out.println(cs.after(cs)); // returns true
  }

/* Implementation of other abstract methods */
}

// The implementation of java.util.Calendar.after() method is shown below
public boolean after(Object when) {
  return when instanceof Calendar && compareTo((Calendar)when) > 0; // forwards to the subclass's implementation erroneously
}

Compliant Solution

This compliant solution recommends the use of a design technique called composition and forwarding (sometimes also referred to as delegation). A new forwarder class that contains a private member field of the Calendar type is introduced. Such a composite class constitutes composition. In this example, the field refers to CalendarImplementation, a concrete instantiable implementation of the abstract Calendar class. A wrapper class called CompositeCalendar is also introduced. It consists of the same overridden methods that constituted CalendarSubclass in the preceding noncompliant code example.

Note that each method of the class ForwardingCalendar redirects to methods of the contained class instance (CalendarImplementation), and receives back return values. This is the forwarding mechanism. This class is largely independent of the implementation of the class CalendarImplementation. Therefore, any future changes to the latter will not break CompositeCalendar which inherits from ForwardingCalendar. When CompositeCalendar's overriding after() method is invoked, it performs the necessary comparison by using the local version of the compareTo() method as required. Using super.after(when) forwards to the ForwardingCalendar which invokes the CalendarImplementation's after() method. In this case, CalendarImplementation's compareTo() method gets called instead of the overriding version in CompositeClass that was inappropriately called in the noncompliant code example, as a product of polymorphism.

Code Block
bgColor#ccccff

// The CalendarImplementation object is a concrete implementation of the abstract Calendar class
// Class ForwardingCalendar
public class ForwardingCalendar {
  private final CalendarImplementation c;

  public ForwardingCalendar(CalendarImplementation c) {
    this.c = c;
  }

  public boolean after(Object when) {
    return c.after(when);
  }

  public int compareTo(Calendar anotherCalendar) {
    // ForwardingCalendar's compareTo() will not be called now	
    return c.compareTo(anotherCalendar);
  }
}

//Class CompositeCalendar
class CompositeCalendar extends ForwardingCalendar {
  public CompositeCalendar(CalendarImplementation ci) {
    super(ci);  
  }
  
  @Override public boolean after(Object when) {
    if(when instanceof Calendar && compareTo((Calendar)when) == 0) // this will call the overridden version i.e. CompositeClass.compareTo(); return true if it is the first day of week
      return true;
    return super.after(when); // does not compare with first day of week anymore; uses default comparison with epoch
  }
	
  @Override public int compareTo(Calendar anotherCalendar) {
     return compareTo(anotherCalendar.getFirstDayOfWeek(),anotherCalendar);
  }

  private int compareTo(int firstDayOfWeek, Calendar c) {
    int thisTime = c.get(Calendar.DAY_OF_WEEK);
    return (thisTime > firstDayOfWeek) ? 1 : (thisTime == firstDayOfWeek) ? 0 : -1;
  }

  public static void main(String[] args) {
    CalendarImplementation ci = new CalendarImplementation();
    ci.set(Calendar.DAY_OF_WEEK, 3); // set the day to the third day (Tuesday in US)
    CompositeCalendar c = new CompositeCalendar(ci);		   		  
    System.out.println(c.after(ci)); // returns false
  }
}

This technique allows a superclass to evolve without causing much distress for its extending classes.

Risk Assessment

Modifying a superclass without considering the effect on a subclass can introduce vulnerabilities. Subclasses that are unaware of superclass implementations can be subject to erratic behavior resulting in inconsistent data state and mismanaged control flow.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

OBJ01-J

medium

probable

high

P4

L3

...

Wiki Markup
\[[SCG 07|AA. Java References#SCG 07]\] Guideline 1-3 Understand how a superclass can affect subclass behavior
\[[Bloch 08|AA. Java References#Bloch 08]\] Item 16: "Favor composition over inheritance"

...

OBJ00-J. Declare data members private      06. Object Orientation (OBJ)      OBJ02-J. Avoid using finalizers