patternjavaMinor
XML parsing and data conversion with Swing
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:
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
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.
-
What is
Try to find a more meaningful name.
-
The widget fields should be
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
-
Comments on the closing curly braces are unnecessary and disturbing. Modern IDEs could show blocks.
“// …” comments at end of code block after } - good or bad?
-
The
-
I'd move the
Then I'd use this class in the
The
-
The
Then the
Your UI have to handle these
-
The
(Unused variable.)
-
I'm not too familiar with Swing, but I think there should be a better way to stop the application than calling
-
I'd use JAXB for the XML processing.
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 methodpublic 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 methodContext
StackExchange Code Review Q#8366, answer score: 3
Revisions (0)
No revisions yet.