Skip to main content

Code comments on JXTable...

No replies
cyberfox
Offline
Joined: 2004-06-22

Greetings,
A few minor comments, from someone who implemented pretty much all of what is in JXTable, and then only now discovered it. (Although JXTable's written to 1.5, which isn't useful to me, so I don't feel as bad as I might.)

To the JXTable author who has a comment about rendering an underline font, you did the right thing. Underlines and strikeouts aren't a font attribute, the way JXTable does it is in fact, the optimal solution as far as I know. (Second best is to use the fact that Swing components can contain HTML, but that can get ugly, and messy in memory usage.)

getSorter(col) is far too cutesy (nested ?:'s!)

{<br />
  if(sorter != null && sorter.getColumnIndex() == convertColumnIndexToModel(columnIndex)) return sorter;<br />
  return null;<br />
}

expresses the intent clearly, and still works because of short-circuiting.

I found the restoreSelection code confusing; specifically the handling of selection.lead.

Firstly, I can only presume that it's doing the dance around selection.lead to preserve the 'last selected' entry. Some commentary to that effect would have helped.

Secondly, it leaves the selection object in a bad state, because it actually changes selection.lead to a random value instead of using a temporary. (This is where I miss const in Java.) While it probably doesn't matter (i.e. it's not triggered by the use-pattern expressed in existing code), a second restoreSelection() with the same selection object could (at random) end up leaving the lead row unselected, add a random row to the selection, or work normally.

I paid a lot of attention to that code because I'm in need of improving my own selection code, so I thought I'd pass along what I'd noticed. (It's the conversion to model-rows and back that I was missing in my own code to handle destructive table updates. Very nice!)

Anyhow, thanks very much for an interesting code-read, and good luck with it. Maybe when I can mandate 1.5 as the minimum (i.e. when 1.6 comes out), I'll be able to use JXTable itself!

-- Morgan Schweers, CyberFOX!