HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

XML parsing and data conversion with Swing

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
conversionwithxmlswingparsinganddata

Problem

This is my first forray into Java Swing and it's not exactly pretty. I have quite a few methods, and a lot of stuff going on (XML parsing, data conversion, etc) and I'm just wondering what kind of general improvements / redundancies I can eliminate from my code.

test.xml:



2012/01/23 00:00:00
Why Did the chicken Cross the road
Because I said So


2012/01/30 00:00:00
ipso lorem 1
I'm Going to get this


2012/02/06 00:00:00
ipso lorem 2
I'm Going to get this


2012/02/13 00:00:00
ipso lorem 3
I'm Going to get this


2012/02/20 00:00:00
ipso lorem 4
I'm Going to get this


2012/02/27 00:00:00
ipso lorem 5
I'm Going to get this


2012/03/05 00:00:00
ipso lorem 6
I'm Going to get this


2012/03/12 00:00:00
ipso lorem 7
I'm Going to get this


2012/03/19 00:00:00
ipso lorem 8
I'm Going to get this


2012/03/26 00:00:00
ipso lorem 9
I'm Going to get this


2012/04/02 00:00:00
ipso lorem 10
I'm Going to get this


2012/04/09 00:00:00
ipso lorem 11
I'm Going to get this


2012/04/16 00:00:00
ipso lorem 12
I'm Going to get this


2012/04/23 00:00:00
ipso lorem 13
I'm Going to get this


2012/04/30 00:00:00
ipso lorem 14
I'm Going to get this




MoreSwing.java:

`package moreswing;
// @author Damien Bell
import java.util.Date;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.io.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JFrame;
import javax.swing.SwingUtilities;
import j

Solution

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

-
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();


-
What is j?

private int j;


Try to find a more meaningful name.

-
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.

-
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.)

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

-
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?

-
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


-
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.)

-
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.)

-
The Scanner instance is unnecessary:

Scanner input = new Scanner(System.in);


(Unused variable.)

-
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



-
I'd use JAXB for the XML processing.

Code Snippets

final Exception x = e.getException();
((x == null) ? e : x).printStackTrace();
private int j;
private final JButton nextButton = new JButton("Next");
...
}// //End ToggleButton method
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

Context

StackExchange Code Review Q#8366, answer score: 3

Revisions (0)

No revisions yet.