Wednesday, March 21, 2012

Ambiguous constructors considered harmful

Eclipse is more than just a code editor: it is a very general and flexible framework for building tools for programmers. I've been using it for my own research in building a tool for debugging cognitive models. But I'm finding that the learning curve for writing good tools in Eclipse is incredibly steep.  I don't want to be too harsh about it because it's an incredible effort involving a lot of people over a lot of years, and maybe a project of that complexity is just necessarily going to trade ease of use for power. But there are at least some incremental ways that the situation can be improved. Here are a couple examples I've found in recent months:

I've been working on a debugging plugin for Eclipse, and learning SWT in the process.  SWT is the library Eclipse uses for drawing buttons and windows on the screen. Today I discovered something horrible in their API. I needed to pick colors for a graph, and I wanted them to be a little darker if they were selected.  The right way to do this is to describe the color in terms of three values: hue, saturation, and brightness, but the Color object in SWT thinks of colors in terms of red, green, and blue.


So how do you convert between the two ways of describing color?  Easy: there's an object called RGB: you load it up with the color you want, and then ask it for the hue, saturation, and brightness values you need.  Here are its two constructors:

RGB(float hue, float saturation, float brightness)
          Constructs an instance of this class with the given hue, saturation, and brightness.
RGB(int red, int green, int blue)
          Constructs an instance of this class with the given red, green and blue values.

If you know H/S/B already and want R/G/B you pass it floating point numbers, then query its member fields; if you know R/G/B already, you pass it integers, and then call a "getHSB" method. As a result, the following code will do something completely insane:

int red = 100;
int green = 100;
int blue = 50;
float pastel = 0.85;
Color normal = new RGB(red,green,blue)
Color bland = new RGB(red*pastel, green*pastel, blue*pastel)

I've seen this in more than one place in SWT: the same class will have multiple constructors with different types, that have different meanings that you'd never guess at, and the types are ones that the compiler is apt to convert automatically for you.  

Another case of this same disease, this time in the JFace library: the story begins with the task of creating three nested widgets: a "Composite" (which is usually a window pane you see as part of Eclipse's interface), a "Tree", which is kind of a spreadsheet-looking control placed in that pane, and a "TreeViewer", an invisible thing between the Composite and the Tree that does some convenient behind-the-scenes accounting work. 

Here's the documentation for a TreeViewer's constructors:

TreeViewer(Composite parent)
          Creates a tree viewer on a newly-created tree control under the given parent.
TreeViewer(Composite parent, int style)
          Creates a tree viewer on a newly-created tree control under the given parent.
TreeViewer(Tree tree)
          Creates a tree viewer on the given tree control.

The first two constructors put a TreeViewer into a Composite, and create the Tree for you automatically.  The third constructor assumes you've already got a Tree inside a Composite, and it sticks the TreeViewer in the middle for you.

The irksome thing here is that a Tree is a subclass of Composite.  In my case, I misread the documentation and tried to pass both a Tree and a "style" argument to the constructor. It matched the second, not the third constructor, treated my Tree as if it were a window pane, and cheerfully put another tree inside my tree (I heard you liked Trees, dawg!). The end result was a Tree viewer managing a different tree than the one I was writing data to, and I spent about a day stepping through SWT code trying to work out exactly where it was going wrong.

There's a growing body of research on how to make APIs usable for programmers. Jeff Stylos' and Steven Clarke's research on constructors, in particular, concluded that it was better (for other reasons) to just have constructors without arguments, and have users set up the objects after their creation with method calls.

In general, making Eclipse easier to write plugins for is a really interesting problem, and I hope after I get over my dissertation hump I'll have time to pitch in and help with it.

1 comment:

Chris Bogart said...

Another example! This time in a method, not a constructor:

The Rectangle class holds just left, right, width and height values.

myRectangle.intersects(otherRectangle) returns a new rectangle that's the intersection of the two.

myRectangle.intersects(x,y,width,height) returns true/false depending on whether your rectangle intersects with the rectangle described by the parameters.

Why use method name overloading for this? I don't get it.