>UI Coding Question

>One of the things I see fairly often in eclipse UI code is something like Tom Schindl‘s example. I could have picked almost any code but Tom was kind enough to post his hack (his words not mine) to the web.


final Tree tree = new Tree(parent,SWT.BORDER|SWT.V_SCROLL|SWT.H_SCROLL);
// Attach a listener directly after the creation
tree.addListener(SWT.Selection,new Listener() {
public void handleEvent(Event event) {
if( event.detail == SWT.CHECK ) {
if( !(event.item.getData() instanceof IPrivileg) ) {
event.detail = SWT.NONE;
event.type = SWT.None;
event.doIt = false;
try {
tree.setRedraw(false);
TreeItem item = (TreeItem)tree.item;
item.setChecked(! item.getChecked() );
} finally {
tree.setRedraw(true);
}
}
}
}
})

One or two of these probably isn’t bad, but how many times are local inline overrides done on events. It could be cleaned up a bit and possibly reused by other listeners within the same method if needed by doing this.


final Tree tree = new Tree(parent,SWT.BORDER|SWT.V_SCROLL|SWT.H_SCROLL);
// Attach a listener directly after the creation
tree.addListener(SWT.Selection,new Listener() {
public void handleEvent(Event event) {
treeToggleCheck(event);
}
})

private treeToggleCheck(Event event) {
if( event.detail == SWT.CHECK ) {
if( !(event.item.getData() instanceof IPrivileg) ) {
event.detail = SWT.NONE;
event.type = SWT.None;
event.doIt = false;
try {
tree.setRedraw(false);
TreeItem item = (TreeItem)tree.item;
item.setChecked(! item.getChecked() );
} finally {
tree.setRedraw(true);
}
}
}

The other alternative is to write a Private or Public class that extends the Listener class and implements the needed functionality. The later option is good when the customized listener is needed to be used in more than one particular case. Eclipse UI code is littered with these inline overrides. There is probably a reason that it’s done this way, and I’m just not understanding it.

Advertisements
This entry was posted in eclipse. Bookmark the permalink.

8 Responses to >UI Coding Question

  1. Gunnar says:

    >There is one problem with your example which Tom’s doesn’t have. It now requires a synthetic accessor for the private method which has some performance implications.

  2. Ed Merks says:

    >How come you didn't complain about the unnecessary nesting of the second if statement when a composite condition using && would have sufficed?

  3. David Carver says:

    >Ed because this post is about inline overrides…I would refactor further to get rid of what you said. :)With that said though…there are times when keeping it inline does make sense. In fact there are times when inline method refactoring should be done.I’m just not convinced that overriding inline methods of classes in the UI makes for readable code. It may make for more efficient runtime code, but shouldn’t those optimizations only be done when it has a significant affect on performance?

  4. >I don’t think there is a deep reason here. I doubt that the method could be called from other listeners. Doing everything inline means that you don’t have to invent a name for the method, and that you can be sure that this is only called from this one place.

  5. Steve says:

    >What is the setRedraw() doing? Get rid of it.Steve

  6. Eric Rizzo says:

    >I with you, Dave. I write a LOT of this kind of UI code and I almost always delegate to a method to do the real work. It’s not always about being able to re-use that method; I just find it a lot more readable when event listener registration is not mixed with event handling implementation.

  7. Eric Rizzo says:

    >To reply to Ed:"How come you didn't complain about the unnecessary nesting of the second if statement when a composite condition using && would have sufficed?"I often find nested if statements to be a lot more readable than complex conditional expressions using && or ||. In this case, I prefer the nested structure as does Tom, obviously. 🙂 Either that, or assign the "complex" part of the expression to a local boolean with a good name and use that in an && expression.

  8. >Simple: Cut’n’paste coding. People see something that works and instead of spending a few seconds on how reuse the existing code, they just copy it. And again. And again. And suddenly, you have 500 copies all over the code, some of them slightly different than others and you don’t know why.And of course, no one cares to push these standard cases back to the UI/whatever framework in question for inclusion, so we could get rid of all of these (or at least all start to copy from the same source).And that, in turn, leads to bug reports like mine when I complained that the table editor generated by data binding is hard to use with boolean values.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s