Madness and the TableModel

This is an pretty old post from my blog, which has been preserved in case its content is of any interest. You might want to go back to the homepage to see some more recent stuff.

What follows is a lengthy rant about a particularly annoying situation in some of my code. Programmers, please let me know - is it the toolkit that is mad, or is it me? Everyone else, feel free to skip it! :)

For one of my current Java projects, I am using a toolkit that comes with its own complete set of GUI widgets based on Swing. Swing… and horror.

I was under the impression that managing a table in any sane OO language goes a bit like this:

  1. Create a class that roughly represents, or at least holds the data for, a row of the table.

  2. Create a “Table Model” or some other kind of backing array to hold objects of that class.

  3. Create a Table widget that uses the table model as its data source.

  4. Happily filter and sort away, knowing that the model and the view are completely separate entities.

And if you should want to serialise the data behind the table to disk, and load it again, you can just save and reload the model, then call some update method on the table itself to let it know that the model has changed.

Bring on the horror!

What this toolkit does, allegedly in the name of MVC, is this:

  1. Create a class that roughly represents, or at least holds the data for, a row of the table.

  2. Create a “Table Model”, but completely ignore it.

  3. Name the columns of the table after the internal member variables of the class you have created.

  4. Add each object to the table in turn, watching in agony as it extracts the values of those member variables, and puts them into the table as Strings, discarding the original object in the process.

But wait, what if those member variables are (sensibly) private? Oh, no worries. It uses reflection, each and every time you add an object to the table, to figure out what the getter method for that variable is called.

And then we come to wanting to serialise the data to disk. Well, that could be a problem - the table doesn’t contain the objects we want, only Strings. Oh, no worries. You can give the table a new instance of your object, have it figure out (again, at runtime) which the appropriate setter methods are, and run those to make your object again!

Oh, hey, I hope that object didn’t contain any variables that weren’t in the table. ‘Cos if so, you’re not getting them back.

Luckily in my case, everything I care about is shown in the table, which only leaves the attempt to serialise it.

Now I want to have a single class that nicely encapsulates this serialising business for all tables, regardless of what the objects expressed in that table are. Normally, one could just serialise the TableModel and be done with it, but now we need to dynamically re-make the objects based on what’s in the table.

For a fully encapsulated solution, I really want to be able to just pass in the table and have the serialiser take care of the difficult stuff. i.e. I want to call:

if (fc.showSaveDialog(this) == JFileChooser.APPROVE_OPTION) {
    SerialisedTableFile f = new SerialisedTableFile(fc.getSelectedFile());
    f.save(table);
}

But I can’t, because f.save() now has no idea what kind of object it is supposed to be building. So we need to pass f.save() a template object of the class that it is supposed to be building, the only requirement of which is that it can be cloned to produce the real object that we want to store data from the table in. So we implement the Cloneable interface – except that Cloneable doesn’t actually include clone() for some no-doubt genius reason. (Not sarcasm, I really do suspect that language designers are an order of magnitude more intelligent than I.)

The end result of all this is that I now have an interface that delights in the name ReallyCloneable, which all classes that I wrangle into tables have to implement. And poor old f.save() looks like this:

public boolean save(Table table, ReallyCloneable itemTemplate) {
    boolean success = false;
    try {
        if (file.exists()) {
            file.delete();
        }
        if (file.createNewFile()) {
            ArrayList<Object> items = new ArrayList<Object>();
            for (int i = 0; i < table.getRowCount(); i++) {
                try {
                    Object o = itemTemplate.clone();
                    table.getItemFromRow(o, i);
                    items.add(o);
                } catch (NoSuchMethodException ex) {
                    Logger.getLogger(SerialisedTableFile.class.getName()).log(Level.SEVERE, null, ex);
                } catch (InvocationTargetException ex) {
                    Logger.getLogger(SerialisedTableFile.class.getName()).log(Level.SEVERE, null, ex);
                }
            }
            ObjectOutputStream out = new ObjectOutputStream(new FileOutputStream(file));
            out.writeObject(items);
            out.close();
            success = true;
        }
    } catch (Exception ex) {
        Logger.getLogger(SerialisedTableFile.class.getName()).log(Level.SEVERE, null, ex);
    } finally {
        return success;
    }
}

I think it’s about time I started buying Bad Code Offsets by Direct Debit.

Comments

aefaradien 04 March 2011

That does indeed seem to be silly and pointless. It not efficient either from a programming or run-time point of view. It looks like someone trying to make their crazy idea stick so that can justify the existence of either the silly idea or them selves. Code like that makes me think egos are involved (though its mostly a guess).

Every GUI system I have seen uses data-source interfaces as providers (SWT, Android, etc.). Storing data in the GUI is a really bad idea... aside from being a PITA to code, it results in data being stored in more than once place. Much better to keep the data in once place and reference it from multiple places - and I Java interfaces are a good way of allowing / controlling this.

Can you measure the cost / impact of this crazy system (run-time efficiency, buggyness of code, developer headaches, etc.) and use that as evidence to get it fixed?

aefaradien 04 March 2011

Its just occurred to me also that the concept of "cloning" objects is a Really Bad Idea (TM). Does a cloned object reference the same children as the parent, or new copies of those? Much tidier to have a factory method(s) for creating new objects of that type. Then the static factory method lives in a class with a private constructor.

The toolkit isn't something I have any power over, so I can't really get it fixed -- however, I can probably avoid using it at certain crucial points.

In response to your second post, it all depends on how you implement clone(). It's not something that your own classes support unless you code the function yourself. In my case the objects I'm cloning only have primitive member variables, so I don't have that worry anyway, but you're right, a factory is a more designpatterny way of doing it.

A colleague of mine suggested a way to use Generics to work around the ReallyCloneable hack as well, so I might even end up with some non-WTF code sooner or later! :)

Add a Comment