[java-gnome-hackers] Major API change in signal interface naming

[Andrew C. <an...@op...>]

Right now, when you connect a signal handler in java-gnome, you do the
following:

        b.connect(new Button.CLICKED() {
            public void onClicked(Button source) {
                // do stuff
            }
        });

        v.connect(new TreeView.ROW_ACTIVATED() {
            public void onRowActivated(TreeView source, TreePath path, TreeViewColumn vertical) {
                // do stuff
            }
        });

given Button b and TreeView v.

This has for some time been the subject of a wee bit of irritation.
Quite evidently, the interfaces which contain our signal handler
callback prototypes are named a bit peculiarly.

The reason it ended up this way is historical; partly the actual signal
names were all caps in java-gnome 2.x (though buried in three layers of
the usual observer pattern Listener/Event verbosity), and partly because
of collisions that occurred when trying to sort out visibility and
naming patterns during the initial java-gnome 4.0 prototype 26 months
ago.

So, for a while now when queried about this my answer was (gee, I'm
happy with the way it is) crossed with "its a bit late to change it now"
seeing as how it was the pattern we inherited from 2.x and have released
in 4.0.

Of course this isn't true; we can change anything we like.

One of the collisions that I had to deal with back when doing the
re-engineering was the problem that there are often methods named the
same as the signal and which cause it to be emitted. An example would be
gtk_button_clicked() which emits the 'clicked' signal, which was
Button's clicked() in our library. So Button.clicked was out as a name
for the signal interface. And it looked ugly too. And since people who
had worked in 2.x were used to 'clicked' being mapped as:

        button.addListener(new ButtonListener() {
            public void buttonEvent(ButtonEvent event) {
                if (event.getType() == Button.Type.CLICKED) {
                     // at last, do something
                }
            }
        });

and for GtkEditable's 'changed' signal:

        entry.addListener(new EntryListener() {
            public void entryEvent(EntryEvent event) {
                if (event.getType() == EntryEvent.Type.CHANGED) {
                    // at last, do something
                }
            }
        });

etc, you can see where the signal name 'clicked' being mapped to a Java
inner type CLICKED came from.

[Those of you new around here be thankful you didn't have to deal with
that mess. Yes it worked, but so painful to write. Completions didn't
work for shit. And trying to navigate the JavaDoc to figure out what
signals were actually available? What a nightmare]

But while Button.CLICKED was something we didn't really trip over, and
indeed provided a nice contrast to the onClicked() method to be
implemented, TreeView.ROW_ACTIVATED just doesn't look right as a Java
type name.

All caps with underscores are _constants_, not types (inner or
otherwise).

++

So you can probably guess where this is going.

With the practise we adopted last year of naming methods that cause a
signal to be emitted with an "emit" prefix, we now have methods like

        b.emitClicked();
        a.emitValueChanged();

for GtkButton's 'clicked' and  GtkAdjustment's 'value-changed'
respectively.

And which really makes it obvious what the java-gnome mapping of the
signal name should have been all along.

[More importantly, the emit prefix mitigates the name collision problem
that I encountered originally...]

++

I am therefore proposing the following API change: signal interface
names in inner types will be the changed to PascalCase matching the Java
conventions for type names.

So we'll now have:

        b.connect(new Button.Clicked() {
            public void onClicked(Button source) {
                // do stuff
            }
        });

        e.connect(new Editable.Changed() {
            public void onChanged(Editable source) {
                // do stuff
            }
        });

        v.connect(new TreeView.RowActivated() {
            public void onRowActivated(TreeView source, TreePath path, TreeViewColumn vertical) {
                // do stuff
            }
        });

taking the examples from above.

The repetition from the first line to the second is _slightly_ annoying
[note 1], but the fact that it clearly and consistently names the signal
is, I think, a massive win.

Before you ask, no, I'm not entirely sure why I didn't consider
Button.Clicked acceptable 2 years ago.

++

You know how these things go. Once you get an idea, you have to try it.
The code generator change was made and a proof of concept done. It
looked pretty damn good. 

And once you get started, it's tough not to finish.

So it's done.

I have a branch called 'signal-name-change' which implements this API
change. The branch is at
bzr://research.operationaldynamics.com/bzr/java-gnome/hackers/andrew/signal-name-change/

All signals in 'mainline' have been converted. Following the deprecation
convention that I wrote about yesterday, all signals that were included
in releases (ie up to 4.0.7) have been duplicated in their original
form, marked /** @deprecated */ [note 2] and have asserts in their
duplicate connect() methods to encourage migration.

I have made the change in one of my projects already, and it was quite
straight forward. Assuming your IDE does not show deprecated methods,
the code completions are perfect and fixing the warnings to do the port
is a few minutes work at most.

So it looks sound.

This went from "well, this is something we could do in the future" to
"wow, this is a big API change and is probably going to have to trigger
4.1.0, damn" to "it's done, and I can get this in before 4.0.8 in an API
safe way" remarkably quickly [note 3].

I've already polled a number of the old heads in our community about
this, and the response has been positive. It is, nevertheless,
appropriate for me to ask for comments here before actually deciding on
this change and merging to 'mainline'.

The branch is there if you want to test it yourself or contribute to the
"clean up the typos" hunt.

AfC
Sydney


1: Trivia: I recently switched my syntax highlighting to have method
declarations be bolded black instead of normal black in my IDE; I did it
to help me pick out method starts a bit more clearly, but it's actually
a nice touch in signal handlers too. I threw in a screenshot of what I'm
talking about.

2. I had to switch from @Deprecated to /** @deprecated */ because
`javadoc -nodeprecated` still includes methods that are @Deprecated when
generating HTML docs. This is annoying.

3: where "quickly" was almost 12 straight hours of hacking. You're
welcome. :)

-- 
Andrew Frederick Cowie

Operational Dynamics is an operations and engineering consultancy
focusing on IT strategy, organizational architecture, systems
review, and effective procedures for change management. We actively
carry out research and development in these areas on behalf of our
clients, and enable successful use of open source in their mission
critical enterprises, worldwide.

http://www.operationaldynamics.com/

Sydney   New York   Toronto   London