Monday, January 31, 2005

Sun to license IntelliJ IDEA?

Well, this sure is an interesting TODO list for JDK 6 ("Mustang") -- from the much larger list:

Application Stack
-----------------
* Buy or cross license a set of commercial applications that provide complete coverage of development needs. (Think MSDN license)
* Tools like JAR to EXE converter, Install Anywhere, IntelliJ IDEA, Atlassian JIRA, JGoodies, database swing controls, etc, should all be included in said Java Developer Network License. Components not necessarily developed by SUN should be included in this developer license sold per seat. (My company buys 1 MSDN license per developer with no known such license in the Java world).
* SwixML / XML Layout mechanism included in an application stack (discussed later)
* Java Help to be included in an application stack (discussed later)
* Critiqued by the community and versions feature frozen with bugfixes and updated regularly to include additional features.

According to Peter Kessler at Sun, this list catches a lot of what Sun is thinking of adding to Mustang (JDK 1.6) or Dolphin (JDK ?).

Cool beans!

Friday, January 28, 2005

Best Quote of the Month

Darren Hobbs states exactly how I feel about growing as a programmer:

I love it when I'm proven wrong, or make a mistake. It means there's still something more to learn.

Oops, null

I often rail against returning null as a marker or default value. I should listen to my own advice!

I'm working on a complex area of a program and deep within its structure is a method for finding the current main window frame. Originally, there might have been several top level frames so they were kept in a global list. But more recently the software moved to using one JVM for each main window, a good approach in light of the strong improvements in JDK 5 with sharing between JVMs.

Enforcing this rule, I wrote this:

public Frame getMainWindow() {
    final List<Frame> list = getTopLevelWindows();

    if (list.size() != 1)
        throw new IllegalStateException(
                "Other than one program on this VM");

    return list.get(0);
}

But this broke automated unit tests which exercise sections of the code calling getMainWindow. Tests do not have a main program window. Ok. So how did I fix it?

public Frame getMainWindow() {
    final List<Frame> list = getTopLevelWindows();

    if (list.size() > 1)
        throw new IllegalStateException(
                "More than one program on this VM");

    return list.isEmpty() ? null : list.get(0);
}

WRONG! I'm now returning null, but what if code is broken elsewhere and the list of top level windows is empty when it should have a member? I've created an NullPointerException at some random location instead of a specific failure. I've ignored my own advice.

What is the correct solution? Well, as the class containing this method implements an interface including getMainWindow and the unit test is already using a delegate instance of the interface, I restore the original version of the method, and implement a test-only version in the testing delegate:

@Override public Frame getMainWindow() {
    assertTrue(getTopLevelWindows().isEmpty());

    return null;
}

I now have correct behavior: running production code demands one and only one top level window per JVM, and unit test code demands none. I wish I had done this in the first place.

Thursday, January 27, 2005

SourceForge.net: XPlanner-IDEA version 0.6.5

Yet another cool plugin for IntelliJ IDEA: XPlanner. The latest improvements (as of 0.6.5):

  • Autopause working task after the specified period of inactivity
  • Open corresponding web page when double-click on task or story
  • Extended task filtering. Ability to view own tasks, own pending tasks, all tasks and not accepted (free) tasks
  • Ability to accept free task or reject own task

IntelliJ IDEA is for me still the best development environment for XP work.

Wednesday, January 26, 2005

Read-only properties in Java

C++ has the advantage over Java of supporting operator overloading. In particular, this makes straight-forward to implement read-only and write-only fields (data members in C++ parlance) that actually call back to methods (member functions) when accessed. C# has this feature as well. You cannot do that in Java, but you can at least provide a form of read-only fields.

Traditional Java uses bean-style getters and setters for field access, but there is a more elegant solution for data transfer-style objects:

public class ReadMe {
    public final String title;
    public final String body;

    public ReadMe(final String title, final String body) {
        this.title = title;
        this.body = body;
    }
}

That's it! This is yet another interesting use of the final keyword. The class fields are public, but because they are marked final, they cannot be modified once set and final fields must be initialized during construction. This prevents tampering, and makes the fields read-only. Using ReadMe is trivial:

public void displayReadMe(final ReadMe readMe) {
    createDialog(readMe.title, readMe.body).show();
}

Trivial — no getters or setters required. This idiom is flawed if you need to update the object, however, but field setters have plenty of drawbacks so this complaint is a mild one.

Monday, January 24, 2005

Throwing complex exceptions

In Java you sometimes have complex exceptions whose creation requires more than just a call to new. In those cases, I like to use a helper method to separate the the construction of the exception from the code which might throw the exception. (In fact my taste is to use such a helper anytime the exception is not ready in a single line of code. Sadly, IOException is such an exception when it has a cause: it is missing the full set of constructors that other descendents of Exception have.)

For such times which of these two snippets is preferred?

public void work(final Queue<PieceOfWork> workQueue)
        throws IOException {
    if (!doWork(workQueue))
        throw createWorkException(workQueue);
}

private WorkException createWorkException(final Queue<PieceOfWork> workQueue) {
    final WorkException e = new WorkException(getJobId());

    for (final PieceOfWork piece : workQueue)
        e.addPieceOfWork(piece);

    return e;
}

Or:

public void work(final Queue<PieceOfWork> workQueue)
        throws WorkException {
    if (!doWork(workQueue))
        throwWorkException(workQueue);
}

private void throwWorkException(final Queue<PieceOfWork> workQueue)
        throws WorkException {
    final WorkException e = new WorkException(getJobId());

    for (final PieceOfWork piece : workQueue)
        e.addPieceOfWork(piece);

    throw e;
}

Until recently I thought there was little difference between them but have changed my mind. Why? The first case is easy for static analysis tools to identify that there are two code paths, one of which throws an exception. The second case requires that the tool descend into throwWorkException and work out that one of the code paths throws an exception.

Although this is not much to ask of many of the more sophisticated static analysis tools, it is too much to ask of most of the tools bundled with common Java editors. IntelliJ IDEA, for example, does not pick up on this as it analysis your code as you type: it makes the sensible tradeoff of performance for completeness.

In fact, shallow static analysis is the reason javac flags this as an error:

public boolean checkCondition()
        throws SomeException {
    if (!cannotProceed())
        throwSomeException();
    else
        return true;

    // Missing return statement
}

Sad code, yes, but not uncommon, especially when the branches get very complicated so that it is not so obvious that they could be simplified.

Lastly, keeping the throw statement as close as possible to where the condition failed helps the most common static analysis tool of all: the human eye. When quickly browing others code, it is easy to miss the true code flow without the helpful throw keyword staring one right in the face.

Thursday, January 20, 2005

Whither aspectj?

This is cool news: the AspectJ and AspectWerkz projects are merging with annotations coming to the fore. I am very curious to see how this will work with JDK 5's apt tool. Apt (annotation processing tool) lets you process annotations at compile-time and extend the compiler with new syntax (as long as that syntax is annotations). I hope aspectj takes advantage of this clever system and uses annotation processors; a cursory look through the compiled jars doesn't seem to indicate that from the class names. It's a shame aspectj's download page does not provide the source to go with the binary installer.

Sunday, January 16, 2005

Wrong method with reflection

Try this:

public Method getUnaryMethod(final Class clazz,
        final String name, final Class parameterType)
        throws NoSuchMethodException {
    return clazz.getMethod(name, parameterType);
}

Looks easy, doesn't it? But:

private static class Overloaded {
    public Class foo(final Number number) { return number.getClass(); }
    // public Class foo(final Integer integer) { return integer.getClass(); }
}

void void testGetUnaryMethod() throws Exception {
    final Class expected = Integer.class;
    final Class actual = getUnaryMethod(Overloaded.class,
            "foo", expected).getParameterTypes()[0];

    assertEquals(expected, actual);
}

The test throws! To pass the test, uncomment the second definition of foo. Unfortunately, I'm working on a dispatch system and this sort of thing is death. Why? Because I cannot repeat with reflection this simple call:

new Overloaded().foo(new Integer(3));

There is a solution, though, replace getMethod with:

public static Method getMethod(final Class clazz,
        final String name, final Class parameterType)
        throws NoSuchMethodException {
    for (Class c = parameterType; null != c; c = c.getSuperclass())
        try {
            return clazz.getMethod(name, c);

        } catch (final NoSuchMethodException e) { }

    return clazz.getMethod(name, parameterType);
}

I look up the inheritance tree for parameterType until I find the first exact match via reflection and return that method. The catch block just lets me retry the lookup one branch higher in the tree. If I exhaust the tree, there is no match even with inheritance, in which case I retry the original lookup so as to throw the same exception as Class.getMethod(String, Class[]) would.

Now I can dispatch dynamically with reflection the same as would the Java runtime. The only drawback is that this simple algorithm only works to vary a single parameter type. For true multiple dispatch, I need a better algorithm to vary more than one parameter type and detect ambiguities: this would let me work methods with more than one argument.

Useless overload

Gotcha! — That's how I felt. I changed code like this:

final Worker worker = new Worker();
final Implement hoe = new Hoe();

worker.work((Hoe) hoe);

To remove the "obviously" useless cast. Well, no, not useless at all. I had overlooked Worker.work():

public class Worker {
    public void work(final Implement o) {
        // Generic work
    }

    public void work(final Hoe hoe) {
        // First do generic work
        work((Implement) hoe);
        // Now do hoe-specific work
        hoe.work();
    }
}

Argh! Of course, there are several much better ways to do this, and I could have changed the declaration of hoe instead of removing the cast. Putting the onus on the caller to pass in the right type of object is just grotesque. And by removing the cast, I broke the code.

I congratulate whomever devised that nasty trap, requiring two useless casts to work correctly and splitting them between the caller and the method no less! Unfortunately, it was purely accidental on their part.

Next time, please code this:

public interface Implement {
    public void work();
}

public class Worker {
    public void work(final Implement implement) {
        // First, generic work
        // Lastly, implement-specific work
        implement.work();
    }
}

Or, if you prefer, make Implement a class with a default, empty work() method. Just something that avoids uselessly useful casts and that requires ESP from the caller.

Thursday, January 13, 2005

The little nullity and a little final

Waldura writes a wonderful article on the final keyword in Java. For me also final is possibly my favorite keyword in the language. What I like best of all about it is its interaction with null. Consider this:

public class Mandatory {
    private final Law law;

    public Mandatory(final Law law) {
        if (null == law)
            throw new NullPointerException();

        this.law = law;
    }

    public Law getLaw() {
        return law;
    }
}

Here law is mandatory, that is, the class never works right without a law. Note the idiom in the constructor. Once the constructor finishes, the class is guaranteed to be null-proof: there's no setter to foul it up; the field is marked final so it won't even compile if you forget to set law in all constructors. No one calling getLaw() need ever check it for nullity. I cannot tell you how tired I get of fixing code where the author does not understand the frequent problems null generates!

And the other way with an optional field instead of mandatory as above:

public class Optional {
    private Virtue virtue;

    public Optional() {
    }

    public Optional(final Virtue virtue) {
        setVirtue(virtue);
    }

    public void setVirtue(final Virtue virtue) {
        this.virtue = virtue;
    }

    public boolean hasVirtue() {
        return null != virtue;
    }

    public Virtue getVirtue() {
        if (null == virtue)
            throw new NullPointerException();

        return virtue;
    }
}

See how much more complicated Optional is than Mandatory? Once you have optional members of a class, it drastically complicates matters. There are extra constructors (and I'm ignoring the whole notion of default values). There is the hasX()/getX() pair to catch accident use of null as early as possible. There is the setter. And we have lost the benefit of final.

The lessons? Avoid optional members (this is quite different, by the way, from the lazy evaluation idiom). Combine final with preventing nullity. The Java language encourages this with the elegance of the mandatory-field class v the clumsiness of the optional-field class.

Write elegant code, not clumsy code.

UPDATE: Reedited for clarity; too much late-night fractured grammar.

Saturday, January 08, 2005

Do or die

I believe that the Do-or-die principle is the strongest protector of good software, and the greatest preventative against mystery bugs, convoluted logic and tortuous coding. What do I mean?

Do-or-die is simple: either an operation succeeds or it raises an exception. When you call a do-or-die method, any normal code path relying on the results of that method have a guarantee for sanity—there are no problems to check for or deal with in the normal code path. The abnormal code path coping with the raised exception handles all problems. (Contrast this with languages lacking exceptions.) This means:

  1. Any procedure always returns void; no boolean returns that require checking for success or failure. Just call the procedure and be done with it.
  2. Any function always returns a valid object; no null returns that require nullity checks. Just capture the return and use it as expected.

Because of do-or-die, tremendous amounts of work, thinking, testing, coding and bug hunting are eliminated. Code is vastly more readable without a thicket of checks, nexted if/else blocks and null handling. Clarity and sanity is a natural product of do-or-die. I can think of no other idiom with as high a payoff in simplicity of notion to size of benefit.

Friday, January 07, 2005

Decreasing functor syntax in Java

Functor, closure, functional, operator, executor: there are many names in Java for the same thing, a class which encapsulates a single generic operation than can be handed around like the unsyntactic anonymous function available in other languages.

For a while, I've used some variation of this model:

public interface Closure {
    void execute();
}

Typical use:

new Closure() {
    public void execute() {
        System.out.println("Ni!");
    }
}

This is perfectly adequate when you create such a thing and pass it around. But what if you are looking to use it immediately? I sometimes see this:

new Closure() {
    public void execute() {
        System.out.println("Ni! Ni!");
    }
}.execute();

The example is contrived, but the principal is easy to grasp: there is a shorter way to do this. Try instead:

public abstract class ImmediateClosure {
    protected ImmediateClosure() {
        execute();
    }

    public abstract void execute();
}

new ImmediateClosure() {
    public void execute() {
        System.out.println("Ni! Ni! Ni!");
    }
};

Shorter. That's all I'm saying.

Tuesday, January 04, 2005

Testing without tests

Like some horrible nightmare come back to haunt the daytime, I'm working on an extensive, mixed-language code base lacking unit tests. XP likes to promote courage—the courage to make changes—, but without unit tests I feel like a tight-rope walker with no net to catch me when I fall. And sure enough, I fell this past week.

Our code base has some serious poor design decisions, many of which are old and embedded in the psyches of existing staff. No one defends the errors, but everyone is afraid to cut them out for fear that the operation may be a success, but the patient dies anyway. I began some cutting last week, trying to untagle a weave of deeply-nested if/else branches, protected data members, fake data objects and everpresent tests for null. Every single one of these is a bad code smell.

It comes as no surprise that I failed to repair all the damage I caused in my changes given that there were no unit tests for the affected code. I added unit tests where I worked directly, but that left large swaths of code untested elsewhere, much of it lurking and unseen in unrelated (but actually related) sections of our system. I've since added more tests and fixed the resultant defects, but in the meanwhile we had about a week of failed Q/A testing in areas no one thought had changed.

XP also eschews code ownership and programmer ego, so I owned up quickly in a public email that I thought I may have caused the problems with my changes, and that I was looking for someone to review code with me to find the trouble spots. This did get one senior programmer to review one of my changed files and to note that he could see nothing wrong with it, but it also prompted him to publically rebuke me when I asked him for a code review of my new changes that fixed one of the problems. He was afraid his name would be tarred by my mistakes. This makes for a difficult environment to work in.

Courage, I remind myself, courage. And more unit tests.