The Java Layer

Moving on to the Java layer, this is the part of the system that makes me cringe. First of all, the documentation is entirely inadequate — page after page of useless default JavaDoc in which nobody has actually bothered to explain what the methods do. The JavaDoc has no examples in it at all. And no examples, anywhere, are explained in depth. The high-level overview document is very sketchy. (You'll learn more from reading the Atom docs —the Java documentation can't stand alone). It's just a mess.

Even worse is the library's structure. Because Google's engineers were thinking in terms of Atom, when it came time to do the Java APIs, they just built a thin wrapper. Consequently, the needlessly complex structure of the Atom feed migrated into the Java. They did build a set of classes that parse the Atom XML, which is good. However, the class that represents a Calendar actually extends the Atom-feed-processing classes, which is really bad.

Google should have abstracted the notion of a Calendar away from the transport mechanism (a "feed"), not effectively said that a calender is a type of feed, which is nonsensical; the way that you get the Calendar data into your Calendar should be unknown. Google's choice of implementing a CalendarFeed (that extends the Atom-feed classes) instead of a Calendar abstraction that uses an "Feed," indicates a fundamental misunderstanding of what data abstraction" means.

There are also practical issues, the main one being a violation of the Liskov Substitution Principle, or LSP (discussed in depth in Robert Martin's excellent book Agile Software Development: Principles, Patterns and Practices). LSP says that you should always be able to use a subclass object as if it's a super-class object, without breaking the behavior of the subclass object.

There's a great simple example of a LSP violation in the Java libraries. Java has a Stack class that extends the Vector class. Stack has the usual methods on it (push(), pop(), etc.) Vector has methods that manipulate a generic sequence of objects. The problem is that a Stack is not a Vector. You can do things to a vector that simply make no sense to a stack. For example, consider:

    Stack<String> s = new Stack(<String>);
    s.push( "a" );
    s.push( "b" );
    s.push( "c" );
    f( s );
    assert s.pop().equals("c");
    assert s.pop().equals("b");  // <--- fails!
    assert s.pop().equals("a");  // <--- fails!

    //...
  public void f( Vector<String> v )
    {   v.remove("b");          // move the middle element to the bottom of the stack!
        v.add( 0, "b" );
    }

It makes absolutely no sense semantically to remove an item from the middle of a stack or to put one on the bottom. More to the point, the caller of f() is in for an unpleasant surprise when it pops items off the stack in a different order than the order in which they were pushed.

So, the modification that f() made to the Stack is both a nonsensical and unexpected operation on a Stack, but it is nonetheless legal Java. That's a LSP violation, and your code should be structured to eliminate dangerous behavior of that sort.

The f() method, itself, might not know it was manipulating a Stack, and if you don't know the surrounding context, f() looks perfectly reasonable. so, the problem isn't in f(). The real problem is that Stack violates the so-called "is-a" test. A Stack is not a Vector in fundamental ways. There's no problem with a Stack using a Vector to organize its data (that is, Stack contains a field of type Vector), but extends is just wrong, here. The extends relationship effectively adds dangerous operations to the public interface of a Stack. If the Stack were to contain a Vector, then those dangerous operations would not be accessible to a user of the Stack.

The problem goes even further in Java, as the Collection interface also contains methods that are unsafe on a Stack (for example, removeAll() and retainAll()), so a Stack can't safely implement Collection, either. This too-fat interface is a violation of another basic design principle: the Interface Segregation Principal (ISP), which Martin also discusses in his book.

Returning to the Calendar APIs and the is-a test, I challenge you to come up with a definition (in an English dictionary) of "calendar" that reads "a representation of a diary, or web-log, entry." The is-a relationship simply does not exist. Nonetheless, Google's Calendar class inherits, incorrectly, from the classes that represent Atom feeds. A Calendar might use an atom feed for marshaling, but it is not an Atom feed. Google's engineers have used an inheritance (is-a) relationship where they should have used a containment (uses) relationship. The practical consequence is that you are mired in the complexity of Atom when all you want to do is access Calendar data or equivalent. The Atom stuff should be completely hidden.

I might be able to live with the LSP violation if I could simply ignore the fact that the Atom-related superclass exists, but I can't. You must use some of the superclass methods to access Calendar-related data in places, and because the documentation is so bad, it's very difficult to figure out which superclass methods are required, which ones are useless, and which ones are actually dangerous.

This bad structure isn't just an academic problem. It translates into many hundreds of hours of real work (and many thousands of dollars of real cost) incurred by every user of these APIs, both when they build and when they maintain their code. Multiply that time and cost by tens of thousands of programmers, and you start to get the picture. Releasing poor quality code of this sort is simply irresponsible. More to the point, this misuse of extends is the sort of mistake that I'd expect in first-year undergraduate work, not in production code from an organization the size of Google.

The other ramification of the incorrect use of extends is that Google has eliminated the possibility of either fixing the XML or using the much better JSON representation in the Java APIs. To do so, they'd have to replace the Atom-related superclass with something else, but all the public methods of those Atom base classes are not only visible to users of the derived class ("clients"), but must be used directly by the clients in places. If Google were to switch to better XML or to JSON, the superclass would have to change, and every existing user of Google's Java APIs (which is forced to use the superclass methods) would have to rewrite their code from scratch. (This is what architects mean by "fragility.") If, on the other hand, Google had provided a Calendar class that abstracted the notion of a "Calendar" away from the Calendar's transport mechanism (if the Calendar used—contained—the Atom-related code internally, but didn't expose it), the migration to JSON would be transparent to the clients; they wouldn't have to change at all.

As a final issue, note that my main critique is that the Atom-related code is exposed at all. I have nothing to say one way or the other about how well (or not) the Atom-related code is actually implemented. A superficial examination of the code shows nothing particularly bad. However, several programmers I've talked to who were working on the same problem and I never got the Java wrappers to work fully. For example, all of us could easily get a list of all of a user's Calendars, but when it came time to get Calendar events, none of us could get anything except a (non-empty) list of seemingly empty Calendar events (no dates; no description; just references to many objects that had nothing in them but null attributes). Could be a bug. Could be that none of us could figure out how to use the classes from the documentation. It's a problem either way.