Return to Revisions

1. Instead of printing stacktraces to the console show the error message to the user. (See later.)

2. The following code with the ternary operator is really hard to read. Try to avoid it.

    final Exception x = e.getException();
    ((x == null) ? e : x).printStackTrace();
   

3. What is j?

    private int j;
   

Try to find a more meaningful name.

4. The widget fields should be private and they could be final. I don't think that other classes should access these objects.

    private final JButton nextButton = new JButton("Next");
    ...
   

It would improve cohesion.

5. Fields should be at the beginning of the class, then comes the constructor, then the other methods. (According to the Code Conventions for the Java Programming Language, 3.1.3 Class and Interface Declarations.)

6. Variable names such as strArray_m does not fit to the Code Conventions for the Java Programming Language, Chapter 9: Naming Conventions)

7. Comments on the closing curly braces are unnecessary and disturbing. Modern IDEs could show blocks.

    }// //End ToggleButton method
   

"// ..." comments at end of code block after } - good or bad?

8. The disableButtonBool parameter of the toggleButton does nothing with the state of the button. I'd remove this parameter.

    public final boolean toggleButton(final JButton buttonDisable, boolean disableButtonBool) {
    buttonDisable.setEnabled(!buttonDisable.isEnabled()); // Swap button
    // state
    if (disableButtonBool) {
    disableButtonBool = false;
    } else {
    disableButtonBool = true;
    }
    return disableButtonBool;// return Bool
    }// //End ToggleButton method
   

9. I'd move the toggle method a ToggleButton class (which would be a subclass of JButton):

    public class ToggleButton extends JButton {
    private static final long serialVersionUID = 1L;
    public ToggleButton(String text) {
    super(text);
    }
    public boolean toggle() {
    final boolean newState = !isEnabled();
    setEnabled(newState);
    return newState;
    }
    }
   

Then I'd use this class in the MoreSwing class:

 private final ToggleButton nextButton = new ToggleButton("Next");
 private final ToggleButton prevButton = new ToggleButton("Previous");

The toggle method belongs here, it manipulates the state of the button. (It increases cohesion.)

10. The xmlLoader method should have a separate class: XmlLoader. Currently it violates the single responsibility principle.

Then the XmlLoader.loadXml() method should throw a custom exception in the catch block:

 } catch (final SAXParseException saxpe) {
 final String errorMsg = "** Parsing error" + ...
 throw new WeekException(errorMsg, saxpe);
 }

Your UI have to handle these WeekException-s. For example, show a Swing error box to the user with the message of the exception. (Find a better name for the exception, please.)

11. The Scanner instance is unnecessary:

    Scanner input = new Scanner(System.in);
    

(Unused variable.)

12. I'm not too familiar with Swing, but I think there should be a better way to stop the application than calling System.exit():

    quitButton.addActionListener(new ActionListener() {
     @Override
     public void actionPerformed(final ActionEvent event) {
     System.exit(0);
     }
    });
    

• How to close a Java Swing application from the code
• This answer also helpful

13. I'd use JAXB for the XML processing.