User interface code smell and update of the wiki

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

User interface code smell and update of the wiki

Arnaud Blouin
Hi all,

Andrey suggested me to send a message here regarding the discussion we
started there:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=510745

We are researchers in software engineering that currently work on user
interface (UI) bad coding (aka. code smell) practices.
We are studying UI listeners, in particular listeners that manage
several widgets, like the following listener we spotted in
org.eclipse.ui.workbench:

@Override
public void handleEvent(Event event) {
      if (event.widget == addResourceTypeButton) {
          promptForResourceType();
      } else if (event.widget == removeResourceTypeButton) {
          removeSelectedResourceType();
      } else if (event.widget == addEditorButton) {
          promptForEditor();
      } else if (event.widget == removeEditorButton) {
          removeSelectedEditor();
      } else if (event.widget == defaultEditorButton) {
          setSelectedEditorAsDefault();
      } else if (event.widget == resourceTypeTable) {
          fillEditorTable();
      }

      updateEnabledState();

}

In such listeners, the source widget is identified using if/switch
statements.
Thanks to empirical studies we have indications that such a practice has
a negative impact on the code quality.
We call UI listeners that control three or more widgets, Blob Listeners
(see this PDF document for more details:
https://hal.inria.fr/hal-01308625v2/document).

We submitted patches to removing Blob Listener instances from
org.eclipse.ui.workbench here:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=510745


Andrey suggest me the idea of adding another section or page to the
Platform UI wiki (https://wiki.eclipse.org/User_Interface_Guidelines)
and describe the blob listener pattern there.
What do you think?


Also, we have several questions for the Eclipse's developers:

1/ Do you think that coding UI listeners that manage several widgets is
a bad coding practice (cf the exemple at the beginning of the email of
the patches)?


2/ Do you think that the threshold of three or more widgets per listener
relevant for characterising a Blob listener?


3/ Do you consider the refactored code suitable for removing Blob listeners?


4/ Do you have any idea of the reason of the presence of Blob listeners
in the code?
For example, lack of anonymous functions (aka lambdas) in Java 7 (and
previous JDKs)? Multiple developers with various experiences,
backgrounds? etc.


5/ Do you have suggestions about other bad user interface coding
practices? This can concern controllers, commands, GUIs, widgets, data
bindings, GUI testing, etc.


Cheers,
--arno



--
Arnaud Blouin
Associate Professor INSA Rennes, France
Inria DiverSE Team (formerly Triskell)
http://people.irisa.fr/Arnaud.Blouin/


_______________________________________________
platform-ui-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/platform-ui-dev

signature.asc (518 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: User interface code smell and update of the wiki

Stefan Xenos-3

I'd argue that listeners themselves are an antipattern.

- They risk memory leaks if they aren't explicitly removed.
- They don't have inherent flow control, and can introduce performance problems when things change more rapidly than those changes need to be consumed.
- They duplicate business logic between whatever code initializes an observer's initial state and whatever updates that state in the listener.
- The set of things that are listened to need to match the set of dynamic data that is accessed within the listener. If they get out of sync, it introduces stale data bugs. Such bugs are hard to test for and easy to create.
- Most listeners are synchronous, which creates all sorts of special cases when they invoke methods that reach back and access the code that is invoking the listener.

Now that we have several good models for reactive programming ( databinding's SideEffect class and tracked getters or one of the various FRP libraries ), there's no need to use listeners at all anymore except within the implementation of said libraries.


On Fri, Jan 20, 2017, 08:43 Arnaud Blouin <[hidden email]> wrote:
Hi all,

Andrey suggested me to send a message here regarding the discussion we
started there:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=510745

We are researchers in software engineering that currently work on user
interface (UI) bad coding (aka. code smell) practices.
We are studying UI listeners, in particular listeners that manage
several widgets, like the following listener we spotted in
org.eclipse.ui.workbench:

@Override
public void handleEvent(Event event) {
      if (event.widget == addResourceTypeButton) {
          promptForResourceType();
      } else if (event.widget == removeResourceTypeButton) {
          removeSelectedResourceType();
      } else if (event.widget == addEditorButton) {
          promptForEditor();
      } else if (event.widget == removeEditorButton) {
          removeSelectedEditor();
      } else if (event.widget == defaultEditorButton) {
          setSelectedEditorAsDefault();
      } else if (event.widget == resourceTypeTable) {
          fillEditorTable();
      }

      updateEnabledState();

}

In such listeners, the source widget is identified using if/switch
statements.
Thanks to empirical studies we have indications that such a practice has
a negative impact on the code quality.
We call UI listeners that control three or more widgets, Blob Listeners
(see this PDF document for more details:
https://hal.inria.fr/hal-01308625v2/document).

We submitted patches to removing Blob Listener instances from
org.eclipse.ui.workbench here:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=510745


Andrey suggest me the idea of adding another section or page to the
Platform UI wiki (https://wiki.eclipse.org/User_Interface_Guidelines)
and describe the blob listener pattern there.
What do you think?


Also, we have several questions for the Eclipse's developers:

1/ Do you think that coding UI listeners that manage several widgets is
a bad coding practice (cf the exemple at the beginning of the email of
the patches)?


2/ Do you think that the threshold of three or more widgets per listener
relevant for characterising a Blob listener?


3/ Do you consider the refactored code suitable for removing Blob listeners?


4/ Do you have any idea of the reason of the presence of Blob listeners
in the code?
For example, lack of anonymous functions (aka lambdas) in Java 7 (and
previous JDKs)? Multiple developers with various experiences,
backgrounds? etc.


5/ Do you have suggestions about other bad user interface coding
practices? This can concern controllers, commands, GUIs, widgets, data
bindings, GUI testing, etc.


Cheers,
--arno



--
Arnaud Blouin
Associate Professor INSA Rennes, France
Inria DiverSE Team (formerly Triskell)
http://people.irisa.fr/Arnaud.Blouin/

_______________________________________________
platform-ui-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/platform-ui-dev

_______________________________________________
platform-ui-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/platform-ui-dev
Reply | Threaded
Open this post in threaded view
|

Re: User interface code smell and update of the wiki

Arnaud Blouin



Le 20/01/2017 à 15:10, Stefan Xenos a écrit :

I'd argue that listeners themselves are an antipattern.


I globally agree, but I prefer to say that listeners may conduct to various bad coding practices when used without caution rather than ban them.

- They risk memory leaks if they aren't explicitly removed.


+1, but data bindings too if the object is not unbound (example: JavaFX).

- They don't have inherent flow control, and can introduce performance problems when things change more rapidly than those changes need to be consumed.


+1. This comes from the fact that current GUI toolkits do not have the concept of user interaction (usually modelled through a finite-state machine): events are produced while interacting with widgets and the developer has to code his own spaghetti code to form a user interaction (example: the mousePressed, mouseReleased, and co, events in Swing).
That is why I develop a GUI toolkit, called Malai, that provides developers with predefined interactions (e.g. DnD, multi-click) reusable in controller/presenter to produce commands. See for instance:
https://github.com/arnobl/latexdraw/blob/master/latexdraw-core/net.sf.latexdraw/src/main/net/sf/latexdraw/instruments/Pencil.java#L297


- They duplicate business logic between whatever code initializes an observer's initial state and whatever updates that state in the listener.


+1. This is a bad coding practice that can be detected using code clone analyses.

- The set of things that are listened to need to match the set of dynamic data that is accessed within the listener. If they get out of sync, it introduces stale data bugs. Such bugs are hard to test for and easy to create.
- Most listeners are synchronous, which creates all sorts of special cases when they invoke methods that reach back and access the code that is invoking the listener.


It seems that the problem here is that listeners do too much. For me a listener should just dispatch the task to do to something which is the job.

Now that we have several good models for reactive programming ( databinding's SideEffect class and tracked getters or one of the various FRP libraries ), there's no need to use listeners at all anymore except within the implementation of said libraries.


I globally agree, but changing legacy code and training engineers take time (some guys will still develop in Swing in 10 years for sure...).
Moreover, if data binding is clearly useful to establish direct mapping between data (as promoted by, for example, WPF), it is not really usable for transactions (i.e., user interacts with widgets to produce undoable actions/commands that modify the domain model of the app).
I did not know ISideEffect. is that the official API to do data binding in Eclipse/SWT?


Cheers,
Arno



On Fri, Jan 20, 2017, 08:43 Arnaud Blouin <[hidden email]> wrote:
Hi all,

Andrey suggested me to send a message here regarding the discussion we
started there:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=510745

We are researchers in software engineering that currently work on user
interface (UI) bad coding (aka. code smell) practices.
We are studying UI listeners, in particular listeners that manage
several widgets, like the following listener we spotted in
org.eclipse.ui.workbench:

@Override
public void handleEvent(Event event) {
      if (event.widget == addResourceTypeButton) {
          promptForResourceType();
      } else if (event.widget == removeResourceTypeButton) {
          removeSelectedResourceType();
      } else if (event.widget == addEditorButton) {
          promptForEditor();
      } else if (event.widget == removeEditorButton) {
          removeSelectedEditor();
      } else if (event.widget == defaultEditorButton) {
          setSelectedEditorAsDefault();
      } else if (event.widget == resourceTypeTable) {
          fillEditorTable();
      }

      updateEnabledState();

}

In such listeners, the source widget is identified using if/switch
statements.
Thanks to empirical studies we have indications that such a practice has
a negative impact on the code quality.
We call UI listeners that control three or more widgets, Blob Listeners
(see this PDF document for more details:
https://hal.inria.fr/hal-01308625v2/document).

We submitted patches to removing Blob Listener instances from
org.eclipse.ui.workbench here:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=510745


Andrey suggest me the idea of adding another section or page to the
Platform UI wiki (https://wiki.eclipse.org/User_Interface_Guidelines)
and describe the blob listener pattern there.
What do you think?


Also, we have several questions for the Eclipse's developers:

1/ Do you think that coding UI listeners that manage several widgets is
a bad coding practice (cf the exemple at the beginning of the email of
the patches)?


2/ Do you think that the threshold of three or more widgets per listener
relevant for characterising a Blob listener?


3/ Do you consider the refactored code suitable for removing Blob listeners?


4/ Do you have any idea of the reason of the presence of Blob listeners
in the code?
For example, lack of anonymous functions (aka lambdas) in Java 7 (and
previous JDKs)? Multiple developers with various experiences,
backgrounds? etc.


5/ Do you have suggestions about other bad user interface coding
practices? This can concern controllers, commands, GUIs, widgets, data
bindings, GUI testing, etc.


Cheers,
--arno



--
Arnaud Blouin
Associate Professor INSA Rennes, France
Inria DiverSE Team (formerly Triskell)
http://people.irisa.fr/Arnaud.Blouin/

_______________________________________________
platform-ui-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/platform-ui-dev


_______________________________________________
platform-ui-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/platform-ui-dev


_______________________________________________
platform-ui-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/platform-ui-dev

signature.asc (518 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: User interface code smell and update of the wiki

Arnaud Blouin
In reply to this post by Stefan Xenos-3

In fact you are talking about model-to-view and view-to-view
synchronisations where data bindings should indeed replace listeners (as
promoted by all modern GUI toolkits).

Blob listeners are not part of that. They are elements of
controllers/presenters/whatever that receive events produced by widgets
to then produce actions/commands that modify the model.



> I'd argue that listeners themselves are an antipattern.
>
> - They risk memory leaks if they aren't explicitly removed.
> - They don't have inherent flow control, and can introduce performance
> problems when things change more rapidly than those changes need to be
> consumed.
> - They duplicate business logic between whatever code initializes an
> observer's initial state and whatever updates that state in the listener.
> - The set of things that are listened to need to match the set of
> dynamic data that is accessed within the listener. If they get out of
> sync, it introduces stale data bugs. Such bugs are hard to test for
> and easy to create.
> - Most listeners are synchronous, which creates all sorts of special
> cases when they invoke methods that reach back and access the code
> that is invoking the listener.
>
> Now that we have several good models for reactive programming (
> databinding's SideEffect class and tracked getters or one of the
> various FRP libraries ), there's no need to use listeners at all
> anymore except within the implementation of said libraries.
>
>


_______________________________________________
platform-ui-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/platform-ui-dev

signature.asc (518 bytes) Download Attachment