The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

Reuse, Liskov, and the Open-Closed Principle

Reuse is the ultimate goal of object-oriented programming: the ability to put an idea into source code and use it later in the same, or similar scenario. The Liskov Substitution Principle states that a subclass should be functionally equivalent to its superclass at all times. Lastly, applying the Open-Closed Principle reduces bugs introduced into the system when adding features because the existing source code should not be modified.

These object-oriented tactics and goals can be achieved with an often overlooked technique. Consider the following class representation of a card game:

public class CardGame extends Game {
  private Deck deck = new Deck();

  public CardGame() {
  }

  private Deck getDeck() {
    return this.deck;
  }
}

public class Deck {
  private List<Card> cards = new ArrayList( 52 );

  public Deck() {
  }

  public Card deal() {
    return getCards().remove( 0 );
  }

  private List<Card> getCards() {
    return this.cards;
  }
}

This code is not reusable, and cannot be extended. The type of Deck used by the CardGame is fixed, as is the number of cards in a deck. A technique to fix the CardGame follows:

public class CardGame extends Game {
  private Deck deck;

  public CardGame() {
  }

  protected Deck createDeck() {
    return new Deck();
  }

  private Deck getDeck() {
    if( this.deck == null ) {
      this.deck = createDeck();
    }

    return this.deck;
  }
}

The above code has two changes from the original. First, the use of lazy initialization to allocate a new instance of the Deck when it is used, and not before. Second, the addition of a createDeck() method that is responsible for how a deck is created. Subclasses of CardGame can override the createDeck() method via polymorphism to return the specific type of Deck for a specific type of CardGame. For example, Euchre is a card game that uses 24, not 52 cards. For completeness, the Euchre class follows:

public class Euchre extends CardGame {
  public Euchre() {
  }

  protected Deck createDeck() {
    return new EuchreDeck();
  }
}

Similarly, the Deck itself must change:

public class Deck {
  private static final int DEFAULT_DECK_SIZE = 52;

  private List<Card> cards;

  public Deck() {
  }

  public Card deal() {
    return getCards().remove( 0 );
  }

  protected List<Card> createCards() {
    return new ArrayList<Card>( getDeckSize() );
  }

  protected int getDeckSize() {
    return DEFAULT_DECK_SIZE;
  }

  private List<Card> getCards() {
    if( this.cards == null ) {
      this.cards = createCards();
    }

    return this.cards;
  }
}

There are a few things going on with the new Deck. First, the addition of a constant and an accessor method for that constant to indicate the number of cards in the deck. Second, like the CardGame, how the cards are created is a method that subclasses can override. The EuchreDeck would look as follows:

public class EurchreDeck {
  public EuchreDeck() {
  }

  protected List<Card> createCards() {
    List<Card> = super.createCards();
  }

  protected int getDeckSize() {
    return 24;
  }
}

How the cards are populated into the deck is a problem for the reader.

The creation technique follows this general pattern:

public class Class {
  private Object object;

  protected Object createObject() {
    return new Object();
  }

  private Object getObject() {
    if( this.object == null ) {
      this.object = createObject();
    }

    return this.object;
  }
}

It isn't a lot of extra code, yet the technique helps address the following issues:

    * Reusability. The objects can be used in different scenarios.
    * Open-Closed Principle. The final renditions of the Deck and CardGame classes need not be modified to create different types of card games.
    * Liskov Substitution Principle. Subclasses can be created that are drop-in replacements for their superclasses.
Dave Jarvis Send private email
Sunday, July 17, 2005
 
 
>  Reuse is the ultimate goal of object-oriented programming

I would say it's not a goal of OO at all. OO is simply a way to structure programs.

> The above code has two changes from the original.

Better to have two constructors, one that takes a deck and one that doesn't. The one that doesn't creates a default deck. Then the accessor doesn't have to do any magik decision making. I generally like RAII approach when possible.

Other than that I like it. Interesting to see generics used in Java.
son of parnas
Sunday, July 17, 2005
 
 
>  Reuse is the ultimate goal of object-oriented programming
"I would say it's not a goal of OO at all."

Ditto (or +1 if you prefer).  In my opinion inheritence for the purpose of code reuse is vastly overrated.  The advantages to me are all your other OOP buzzwords: Abstraction, Polymorphism, and Encapsulation.

Reuse to me is a side benefit to a well made class hierachy that's real effect is usually rather insignificant.  After all, the point of a specific class is to provide an implementation that is different from its base... otherwise why have the class.
Mark Flory Send private email
Sunday, July 17, 2005
 
 
Another +1 on OP being wrong with the first statement of the post.

Also, there is little benefit to talking about class design in the abstract; you can only speak in generalizations.
Jonas Quimby Send private email
Sunday, July 17, 2005
 
 
Actually, I think you guys are putting things upside down, putting the cart before the horse, however you want to say it. 

Reusability is in fact a primary goal.  Abstraction, Polymorphism, and Encapsulation are goals only because they help realize the ultimate goals of programming, one of which is reusability. 

Abstraction, Polymorphism, and Encapsulation are merely subgoals.  They judged as good things only because they enable, among other things, reusability.

Reusability plays a big part of the development of an OO method in Bertrand Meyer's "Object Oriented Software Construction".  In the very first chapter he identifies the factors "whose pursuit is the central task of object-oriented software construction":  Correctness, Robustness, Extendibility, Reusability, Compatibility, Efficiency, Portability, Ease of Use, Funtionality, Timeliness.  These factors often compete with each other, but Reusability figures in his book as one of the most important of his named overarching goals of OO.
Herbert Sitz Send private email
Sunday, July 17, 2005
 
 
Actually I may have just stated things a little too strongly.  In Meyer's discussion of Reusability I don't think he was speaking specifically of code reuse.  He was thinking more in terms of reuse of objects (or components or widgets) from different applications, without the need to access or even see the source code of the object or component you're reusing.  This is necessarily tied to code reuse, even if code reuse was not one of his stated goals.
Herbert Sitz Send private email
Sunday, July 17, 2005
 
 
I'd read somewhere recently--maybe someone else will recognize it and attribute it properly--that the special twist OO brings to "code reuse" involves having *old* code call into *new* code, rather than the other way around.

When I'd first heard that, in Zen terminology, I felt as though I'd glimpsed the tracks of the bull.  YMMV.  :-)
Mike Bland Send private email
Sunday, July 17, 2005
 
 
Hi,

When you have multiple constructors you enjoy the following losses:

1. The ability to have Spring auto-create and populate objects.
2. Autonomous classes.

The autonomy issue is rather subtle, IMO. When an object is created, if it has different types of parameterized constructors, then the clients of that class are coupled to it vicariously through those parameters. In other words, an absolute dependency is created. Whereas providing set accessors to give an object the information it may (or may not!) need to perform its tasks yields a looser coupling.

For example:

public class Authenticator {
  public Authenticator( String user, String password ) {
    setUser( user );
    setPassword( password );
  }

  public void login() throws AuthorizationException {
  }
}

I would prefer:

public class Authenticator {
  public Authenticator() {
  }

  public void login() throws AuthorizationException {
  }

  private String getUser() throws AuthorizationException {
    if( this.user == null ) {
      throw new InvalidUserNameException();
    }

    return this.user;
  }
}

This is what I mean when I say "autonomy". Clients of the Authenticator class no longer need to create it with a username and password. They simply create the class and start using it. If the Authenticator determines that it cannot perform the requested behaviour (i.e., "login"), then it tosses an exception to indicate the problem.
Dave Jarvis Send private email
Sunday, July 17, 2005
 
 
Dave: Actually, I believe the second is far worse since the first example ensures that the object is constructed in a valid state.

With the second option, the caller needs to know the internals of the class to be able to determine how the object should be set up to be in a valid state and, potentially, how to use the object.

To deal with the problem of having giant constructors and / or tens of constructors for different possible combinations, use a Builder pattern.

(my 2 cents)
Andrew Knock Send private email
Monday, July 18, 2005
 
 
> The ability to have Spring auto-create and populate objects.

So much for spring not interfering with your POJOs. It's rather weak argument that I shouldn't use RAII because of spring.

> then the clients of that class are coupled to it vicariously through those parameters.

That would be the point, as the client, if it is creating the object, is in the position of knowing the types.

If the client is not creating the object then it should already be created and be avaialable from a factory or singleton or something. Having accessors magically create member objects is a rather poor practice imho. It redundantly embeds the information on determining the contained type. Better decoupling happens when an interface is used.
son of parnas
Monday, July 18, 2005
 
 
Hi, Andrew.

The parameterized constructor of the Authenticator does not guarantee its instances are in a valid state:

public class AuthenticatorHTTPClient {

  public void doLogin( String login, String password ) {
    Authenticator a = new Authenticator( login, password );

    // Oops! NullPointerException is thrown here, because the
    // caller of doLogin used null as a parameter. It really
    // should throw a checked AuthenticationException
    a.login();
  }
}

Again, autonomy, I believe, is the key. That means ensureing that your objects can be used anytime, and handling error conditions gracefully. This precludes them from being in "invalid state": there should be no such thing.

As a couple real-life examples:

Ariane 5 tried to convert a 64-bit to 16-bit number. Conceptually, this is putting an object into an invalid state, then trying to use it. The result was a $7B mistake.

http://www.around.com/ariane.html

The Mars Orbiter had an uncorrected unit-conversion error, that could have been caught and fixed manually. The software was not robust enough to handle being used in an invalid state. $183M mistake.

http://www.tcnj.edu/~rgraham/failures/MCOcraft.html

I'm not advocating writing perfect code; rocket scientists are rather clever folks. I'm advocating writing objects that are as autonomous as possible by ensuring that even if their internal data is uninitialized (or invalid), that they do not do anything "unexpected" (e.g., NullPointerException).
Dave Jarvis Send private email
Monday, July 18, 2005
 
 
If you shouldn't get an exception when you try to operate on an object that is in an inconsistent state, then what *should* happen?
Confused now
Monday, July 18, 2005
 
 
You shouldn't get an *unexpected* (unchecked) exception. An uncaught NullPointerException (ArrayIndexOutOfBoundsException, RuntimeException, etc.) is bad. A checked AuthenticationException is better because the compiler ensures that the exceptional case is being handled by the caller.
Dave Jarvis Send private email
Monday, July 18, 2005
 
 
Ah, okay, I see what you mean now.  Thanks for clearing that up.
Confused no more
Monday, July 18, 2005
 
 
lint warns about possible use of uninitialised variables etc.

Other tools might be even better.
new nick, new rep
Monday, July 18, 2005
 
 
I agree that a RuntimeException variant is bad (usually), but not because it's uncaught.  It's bad because it indicates a programming error. 

The reason why RuntimeException doesn't have to be caught is because the program should only have to handle expected states, and be protected by the VM when the unexpected happens.  This isn't to say that a programmer shouldn't try and avoid runtime errors, but that planning for every possible contingency --expected and not-- doesn't solve the problem of the code being incorrect.
Matt Brown Send private email
Monday, July 18, 2005
 
 
"Having accessors magically create member objects is a rather poor practice imho."

Since we're on the subject of Liskov substitution, Open-Closed principle, and whatnot, having an accessor that lazy initializes a member seems to me to be in violation of the Command/Query separation principle [http://en.wikipedia.org/wiki/Command-Query_Separation]

I'm not saying that it's a bad thing since I do it myself from time to time, but it does seem to be a genuine violation of the principle.
From the peanut gallery
Monday, July 18, 2005
 
 
Hi, Peanut Gallery.

"principle, and whatnot, having an accessor that lazy initializes a member seems to me to be in violation of the Command/Query separation"

Lazy initialization is referentially transparent, and consequently does not violate Command/Query separation.

http://en.wikipedia.org/wiki/Referential_transparency

In other words, no matter how often you call "getObject", it will always return a valid object (never null). The only time it could produce unexpected results is when you do:

instance.setObject( null );
object o = instance.getObject();

In this case, "o" isn't null (which is expected). However, since that would also break the contract of getObject() (@post never returns null), I feel we can deem it moot.
Dave Jarvis Send private email
Monday, July 18, 2005
 
 
> "principle, and whatnot, having an accessor that
> lazy initializes a member seems to me to be in
> violation of the Command/Query separation"

I am not really all the concerned what principles are violated. I'll violate principles if it helps me.

The problems I see are:
1. The get object isn't thread safe. Two calls from different threads will return different decks.
2. Add a sync makes the program less performant.
3. Adding all the wrapper code consuses what should be simple.
4. I'd rather use separate testable factories so a class using the deck abstraction is free of creation issues. That's simpler and clearer to me.
son of parnas
Monday, July 18, 2005
 
 
"In other words, no matter how often you call "getObject", it will always return a valid object (never null)."

But it could violate the class contract if, within the class, you access the storage for the member in some other method.  For instance, if you had an isObjectLoaded() query that returned false if the storage was nulled out, then calling getObject would leave behind an observable side-effect.

I see what you mean though.  It's not the lazy initialization that causes the violation, it's the failure to use a single access channel to the storage that causes the violation.  Thanks.
From the peanut gallery
Monday, July 18, 2005
 
 
Hi, Son of Params.

"1. The get object isn't thread safe. Two calls from different threads will return different decks."

Make the call synchronized; or synchronize the conditional expression. If you're worried about performance, use C++. If you have to use Java and are worried about performance, then make it synchronized and use a profiler to find your bottlenecks.

Hitler and premature optimization are the root of all evil.

"4. I'd rather use separate testable factories"

Factories are a good alternative. What I find, though, is that it is not always appropriate to use a factory when creating something like a an ArrayList, or you have several different objects (e.g., Key, Door, Window, Lock, Hinge, KittyDoor, Material) then creating many different Factories would be overkill.
Dave Jarvis Send private email
Monday, July 18, 2005
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz