Sunday, April 12, 2020

Fixing Swing's JOptionPane class

I maintain a handful of client-side Java programs that use Java's older GUI API called Swing. One thing that I have found awkward about Swing is the way dialog boxes are handled.

Swing provides a class called JOptionPane for creating simple dialog boxes. It alleviates the need to manually code your own, which saves a lot of time. All you have to do it make a single method call and your dialog appears.

While the class works just fine, some of its methods have up to eight parameters. When a method has more than three or four parameters, it makes it hard for someone who is not already familiar with the API to tell what the code does at a glance.

For example, the code below has a total of eight parameters. Unless you are already familiar with this method's signature, you probably don't have a clue as to what half of these parameters do.
int choice = JOptionPane.showOptionDialog(
    this,
    "The password you have chosen may not be secure.\n\n" +
      "Please keep the following guidelines in mind:\n\n" +
      "1. Eight or more characters long.\n" +
      "2. Is not a word from the dictionary." +
      "3. Completely different from all your other passwords.\n\n" +
      "Would you like to create a better password or continue?",
    "Security warning",
    JOptionPane.YES_NO_OPTION,
    JOptionPane.WARNING_MESSAGE,
    null,
    new Object[]{"Pick a new password", "Continue"},
    "Pick a new password"
);
One thing you could do to make the code more readable is assign each parameter to a variable with a descriptive name. This allows a programmer who is not familiar with the method to see at a glance what each parameter is for.
Component parentWindow = this;
String text = 
    "The password you have chosen may not be secure.\n\n" +
    "Please keep the following guidelines in mind:\n\n" +
    "1. Eight or more characters long.\n" +
    "2. Is not a word from the dictionary." +
    "3. Completely different from all your other passwords.\n\n" +
    "Would you like to create a better password or continue?";
String title = "Security warning";
int buttons = JOptionPane.YES_NO_OPTION;
int messageType = JOptionPane.WARNING_MESSAGE
Icon icon = null; 
Object[] buttonLabels = new Object[]{"Pick a new password", "Continue"}
Object defaultButton = "Pick a new password";

int choice = JOptionPane.showOptionDialog(parentWindow, text, title, buttons, messageType, icon, buttonLabels, defaultButton);
However, there are still a some problems with this:
  1. Variable name conflicts: The variable names could be confused with other variable names in the same scope. For instance, the name "text" is fairly generic, so it's not unlikely that another variable a dozen or so lines down could have the same name.
  2. Parameters are ordered: It is very easy for the programmer to get the parameter order wrong without the compiler noticing. For example, if the "buttons" and "messageType" variables were swapped, the compiler wouldn't notice. The programmer likely wouldn't notice either because they are next to each other in the method signature.
  3. Null parameters: One of the parameters is null. The code would be more concise if we could just leave it out, but the JOptionPane class does not provide an appropriate method signature for that.
  4. Misleading code formatting (1): The way the message text string is formatted in the code makes it look like each line will end with a single line break. However, some lines end in two line breaks.
  5. Misleading code formatting (2): Each line of the message appears on its own line in the code. This makes it much easier to read! However, you may not have noticed that the fourth line is missing a line break. Because of the way the code is formatted, an error like this can easily be overlooked.
  6. Neglecting native system settings: The code may not be using the native system's preferred line break sequence. Appending "System.lineSeparator()" (or even a concisely-named "newline" variable) onto each line would be technically correct, but it would make the code less consice.
  7. Duplicate data: One of the button labels is duplicated.
  8. Ambiguous types: Defining button labels as Objects will lead to problems only detectable at runtime if an Object that JOptionPane does not accept is passed into the method (also, it's just weird!).
To resolve these issues, I designed a class that acts as a fluent wrapper around JOptionPane. Using this wrapper class, the above code would look like this:
int choice = DialogBuilder.warning()
    .parent(this)
    .title("Security warning")
    .text(
        "The password you have chosen may not be secure.",
        "",
        "Please keep the following guidelines in mind:",
        "",
        "1. Eight or more characters long.",
        "2. Is not a word from the dictionary.",
        "3. Completely different from all your other passwords.",
        "", 
        "Would you like to create a better password or continue?")
    .buttons(JOptionPane.YES_NO_OPTION, "*Pick a new password", "Continue")
.show();
The above problems are addressed as follows:
  1. Variable name conflicts: No variables are needed because the method name provides context. 
  2. Parameters are ordered: Each parameter is replaced by a method call. Methods can be called in any order.
  3. Null parameters: The class internally provides a default value for every optional parameter. The method call for this null parameter can be left out, making the code more concise.
  4. Misleading code formatting (1): The "text()" method takes a single vararg parameter, where each parameter is a line of text. If a line ends in two line breaks, an empty string is provided, making it clearer that an empty line will appear on the dialog box.
  5. Misleading code formatting (2): Line breaks are automatically added by the "text()" method.
  6. Neglecting native system settings: The "text()" method uses the native system's preferred line break sequence.
  7. Duplicate data: The default button is identified by an asterisk character, which is removed when the button label is passed into the JOptionPane class. This is not as obvious as I would like it to be, as it requires detailed reading of the Javadocs, but it is very concise.
  8. Ambiguous types: The button labels are defined as Strings in the "buttons()" method, not Objects. This means the compiler can prevent invalid objects from being passed in. 
The full class can be viewed here: https://github.com/mangstadt/emc-shopkeeper/blob/master/src/main/java/emcshop/gui/DialogBuilder.java

No comments: