EOB 1.0

Somewhat late in the day I know, but in case you didn’t already know, EOB has reached 1.0. Congratulations.

Advertisements

What if all methods were objects?

What would happen if you wrote code where every method of an object simply delegated to a field? Where every method was essentially a ‘Method Object’ as described in the Refactoring book. What would that be like? Objects would become like mini-packages. Loose aggregations of behaviour collaborating to form a whole like cells in an organism. What would be the advantages and flaws? I haven’t tried it so I don’t know for sure. One advantage I can see is that of testability. If all your methods are objects, testing a private method becomes simply a matter of instantiating its object and invoking it. It would also make it harder to write code with side effects. Multiple return values (ie. methods that modify several fields) might be tricky though. On the other hand this might force more careful thought about where behaviour should be.

Food for thought on a friday…

Putting the Unit in Unit Tests

Some excellent comments to my last post. Please allow me to respond:

Should you test your persistence layer? Yes.
Should you test your system from end to end? Yes.
Should you test complex edge cases? Yes.

If you assumed the answer to any (sensible) question that starts with ‘Should you test…’ is ‘Yes’, you wouldn’t be too far from the truth.

However….

Should you test any of those things in your Unit tests? No. Unit tests (to me) are the things that the developers write and run many times a day, and should always pass. Testing against DB’s is hideously slow. Orders of magnitude too slow for unit tests. Tests against DB’s can too easily fail for the wrong reasons. Tests that require masses of setup code and mock objects just to run are too brittle. Lots of mock objects can hurt you when refactoring, as they tend to break when you pull their superclasses apart.

Also note that I didn’t say ‘Don’t use Mocks’. Sometimes they are required. MockPersistenceLayer being a good example. Although if you have interface/impl separation (which you should for your persistence layer), then ‘InMemoryPersistenceLayer’ or ‘HashMapBackedPersistenceLayer’ would be equally good names, and less open to misinterpretation than MockPersistenceLayer.

The other kind of Mocks, the ones that verify their methods were called in a certain sequence with certain parameters can be useful, but, as I said, can be brittle and prone to breakage during refactoring, so should be used with caution. They also make it less clear what’s being tested. Chances are the system would benefit from finer grained objects that could be tested individually, which should reduce the need for a full Mock.

Its perfectly ok to have a separate set of JUnit tests that aren’t ‘Unit’ tests, that (for example) test the persistence of each of your objects exactly once. Run them as often as you want, add them to your commit script if you feel the need, just don’t bolt them into the main body of unit tests.

Project guidelines

Just some thoughts, mostly for my own benefit. I reserve the right to change my mind, and even do complete U-turns where necessary.

  • Use CruiseControl, or some other form of continuous integration.
  • Use clover, and fail checkins if there is any code not covered by a unit test.
  • Block System.out.println with a CVS commit script.
  • Ban unit tests from hitting the database.
  • Don’t write tests that require more than 10 lines of (non production) setup code per test method.
  • Don’t write code that requires a (JUnit) setUp method that is bigger than all your test methods combined.
  • Use mock objects very sparingly. Consider them indications of code smell.
  • Don’t use anonymous subclasses in testcases just to loosen method permissions for testing, or to stub out hard-to-test behaviour. Code smell warning.
  • Do put your test code in a parallel directory structure, in the same packages as your production code.
  • Do test protected and package methods.
  • Don’t worry about testing private methods too much – they must be called from a public, package or protected method, or they’re redundant and can be deleted.

Don’t break the law of Demeter

…Or the OO cops will get you. The best summary of the law of demeter I saw was from c2.com. It goes something like this:

You can play with yourself.
You can play with your own toys (but you can’t take them apart),
You can play with toys that were given to you.
And you can play with toys you’ve made yourself.

That’s it. To recast in geek-speek:

An object can call its own methods.
An object can call methods on its own fields. (But not on it’s fields’ fields)
An object can call methods on parameters passed to it.
An object can call methods on objects it has instantiated.

That’s it. You can’t call methods on the fields of other objects. So anything that looks like foo.getBar().getBaz().doSomething() is not allowed. What you can do is add a method to foo called ‘doSomething’ which delegates to a method on its ‘bar’ field called ‘doSomething’ which delegates to its ‘baz’ field. That doesn’t break the law, because it prevents foo’s caller from knowing about bar and baz, and foo from knowing about baz.

Its all about preserving encapsulation.

If you take it to the extreme you find that getter methods break the law of demeter. Ask yourself why other objects need to get at your object’s fields. Can’t they just tell your object what they want it to do directly? Setter methods aren’t much better. They still expose an object’s state to the world.

Something new every day

IDEA is just the coolest IDE since sliced bread. Today I discovered the ‘invert if’ and ‘split if’ refactorings. Just put the text cursor next to an ‘if’ statement and IDEA will ask if you would like to invert it or split it! Consider the following:

if (foo != null  && bar != null) {
// Do some stuff
return true;
}
return false;

After ‘split if’:

if (foo != null) {
if (bar != null) {
// Do some stuff
return true;
}
}
return false;

After two ‘invert ifs’:

if (foo == null) {
return false;
}
if (bar == null) {
return false;
}

// Do something
return true;

Which I think is much clearer and more open to further refactorings, as it has clearly separated the guard clauses from what the method is actually supposed to be doing. Cool.