Skip to main content

Writing Great OO Code Day One

Posted by thedarksavant on October 3, 2008 at 11:23 AM PDT

There's no shortcut to experience. Writing good object oriented code takes experience, but here are three practices to help you get off on the right foot day one with even the grumpiest of gray beards:

  1. Write all your code using Test Driven Development(TDD)
  2. Follow the Rules of Simplicity
  3. Tell Don't Ask

Write All Your Code Using TDD

Code written test first and code written test last is very very different code. Code written test first is loosely coupled and highly cohesive. Code written test last often breaks encapsulation when some property or private method needs to be exposed to the test because the class wasn't designed to be tested. If you write your code test first, your dependencies will be better and your code will be loosely coupled and highly cohesive. More on how tests help you design better code later.

Follow the Rules of Simplicity

Code is simple when it:

  1. Runs all the tests
  2. Contains no duplication
  3. Expresses all the intent
  4. Utilizes the fewest classes and methods

It's important to notice that I've used an ordered list. The order is important. A single GodClass with a single main() method is not simple. It may run all the tests, but in any program more complex than "Hello, world!" it certainly will contain duplication and not express all the intent.

My struggle with the Rules of Simplicity focused around the If Bug. I didn't understand how following the rules of simplicity would head off someone writing if heavy code. One could argue, and I tried, that if heavy code does not express intent. But when you read code like

if (mobile.getType() == MobileTypes.STANDARD) {
  alert();
}

It's pretty darn easy to see the intent. In the context of whichever method this is in, if the mobile is of type STANDARD then alert. How much more intent do you need?

Then I had a little epiphany. If there's code like that, then certainly in other places in the code there's more. There's probably code like:

if (mobile.getType() == MobileTypes.GAS) {
  registerGasReading();
}

and
if (mobile.getType() == MobileTypes.TEXT) {
  sendTextMessage();
}

and
if (mobile.getType() == MobileTypes.LOCATION) {
  notifyLocation();
}

Do you see it? I sure do. Violations of rule 2. Many many violations of rule 2. And the worst kind of violations of rule 2. Duplication in many different pieces of the code. Duplication that is going to be very very hard to find. So to help prevent this, I've included

Tell Don't Ask

Tell Don't Ask simply means don't ask an object about it's state and then do something. Tell the object to do something. This means all those if examples become:

mobile.alert();

and
mobile.registerGasReading();

and
mobile.sendTextMessage();

and
mobile.notifyLocation();

Now suppose that there were some of those if clauses splattered throughout the code that had duplicate implementations. In the if heavy version, they would be very hard to find, but in the tell don't ask version, all the implementations are in Mobile. All in one place and ready for sniffing out and eradicating.

Listening to your tests will also help you keep your code simple.

public interface Alarm {
  void alert(Mobile mobile);
}

public class Siren implements Alarm {
  public void alert(Mobile mobile) {
    if (mobile.getType == MobileTypes.STANDARD) {
      soundSiren();
    }
  }
}

public class TestSiren extends TestCase {
  public void test_alert() {
    LocationMobile mobile = new LocationMobile();
    Siren siren = new Siren();
    siren.alert(mobile);
    assert(sirenSounded());
  }
}

If you listened closely to your test, it would be asking you, "Why do you need a LocationMobile to test the Siren?" Why indeed? It seems that Sirens shouldn't even know about LocationMobiles.

public class LocationMobile {
  private Alarm alarm;
  public LocationMobile(Alarm alarm) {
    this.alarm = alarm;
  }
  public void alert() {
    alarm.alert(); // alert on Alarm no longer needs a mobile
  }
}

public class TestLocationMobile() extends TestCase {
  public void test_alert() {
    Alarm alarm = EasyMock.createMock(Alarm.class);
    alarm.alert();
    EasyMock.replay(alarm);
    Mobile mobile = new LocationMobile(alarm);
    mobile.alert();
    EasyMock.verify(alarm);
}

It looks like I've just swapped the dependencies. Instead of Alarm depending on Mobile, I now have Mobile depending on Alarm. But if you look closely at the test, the real dependency was Siren knowing about LocationMobile. A concrete class depended on another concrete class. This violates the Dependency Inversion Principle(DIP). The second example has LocationMobile depending on the interface Alarm. A concrete class depending on an abstraction. This satisfies DIP.

If you write all your code TDD, follow the Rule of Simplicity, and Tell Don't Ask then you'll be on the path to becoming a better OO programmer. Good OO code is easy to read and maintain, but can be hard to write. At least at first. The more you write the better you'll become, and the more experience you will get. In the meantime, these practices should get you well on your way.

Related Topics >>

Comments

I think there is a mistake in the example code. There was no dependency between Siren and LocationMobile. That is actually an example of a bad test code: better use "mocks" of the interfaces instead of real implementation classes.

Thanks, DW. Unfortunately I do not know of any open source projects which are written with this philosephy. A few years ago I poked around the code of XPlanner, and it's code was very close, but it's been a few years and there have been many contributors, so I'm not sure of the state of the code today. Thanks for reading!

Very nice article, Curtis. Do you know of any open source projects (not very big, so the code code can be easily grokked) which have such a design and code written in such a modular way)? Thanks -DW

The dependency between Siren and LocationMobile is implied, because the easiest way to get if (mobile.type == MobileTypes.STANDARD) to be true is to pass in a LocationMobile to the alert method. The real point is that asking a mobile what type it is then doing something based on the type is a violation of Tell Don't Ask. Perhaps if the code was written like: if (mobile instanceof LocationMobile) { soundAlert(); }

Whole heartedly agreed, especially with Rule #1. For a product I'm working on, we adopted coding guidelines that every class that could live in its own NetBeans project would be split off into its own project, complete with individual test code. Our reason for going this far? From my experience, the greater the number of classes in a project, the less test code actually gets written. People naturally start using the end application as their testbed, which IMO is bad practice since it leads to brittle, single-purpose, under-tested code. Our policy is that code isn't used in the product until it has a test harness, and we don't use the application for testing. Instead when we find bugs, we duplicate them in our test code and fix them there. This has been a difficult cultural shift. About half my developers don't see the practical side of writing test code and doing as much development as possible outside of the application. The benefit in my opinion, is huge. Since each class is written as an independent tool rather than as a cog in a larger solution, the code is easy to use in other applications, it's easier for larger teams to work on, its easier to learn since the test code succinctly demonstrates how the code should be used, code has the benefit of being exercised in different environments (both the test environment and the more complex application environment), etc. This shift hasn't been all joy and sunshine. NetBeans doesn't gracefully handle hundreds of projects, and since ant does an exhaustive tree walk building hundreds of projects can take some time. (We've written a new ant task that fixes that problem.) But IMO the plusses outnumber the minuses by a large margin.