XML parsing and data conversion with Swing

Question

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:

<weeks>
    <week>
        <date>2012/01/23 00:00:00</date>
        <riddle>Why Did the chicken Cross the road</riddle>
        <lastWeekSolution>Because I said So</lastWeekSolution>
    </week>
    <week>
        <date>2012/01/30 00:00:00</date>
        <riddle>ipso lorem 1</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/02/06 00:00:00</date>
        <riddle>ipso lorem 2</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/02/13 00:00:00</date>
        <riddle>ipso lorem 3</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/02/20 00:00:00</date>
        <riddle>ipso lorem 4 </riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/02/27 00:00:00</date>
        <riddle>ipso lorem 5</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/03/05 00:00:00</date>
        <riddle>ipso lorem 6</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/03/12 00:00:00</date>
        <riddle>ipso lorem 7</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/03/19 00:00:00</date>
        <riddle>ipso lorem 8</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/03/26 00:00:00</date>
        <riddle>ipso lorem 9</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/04/02 00:00:00</date>
        <riddle>ipso lorem 10</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/04/09 00:00:00</date>
        <riddle>ipso lorem 11</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/04/16 00:00:00</date>
        <riddle>ipso lorem 12</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/04/23 00:00:00</date>
        <riddle>ipso lorem 13</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
        <week>
        <date>2012/04/30 00:00:00</date>
        <riddle>ipso lorem 14</riddle>
        <lastWeekSolution>I'm Going to get this</lastWeekSolution>
    </week>
</weeks>

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 javax.swing.JPanel;
import javax.swing.JButton;
import javax.swing.JTextArea;
import javax.swing.JLabel;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.DocumentBuilder;
import org.w3c.dom.Document;
import org.w3c.dom.NodeList;
import org.w3c.dom.Node;
import org.w3c.dom.Element;
import java.util.Scanner;
import org.xml.sax.SAXException;
import org.xml.sax.SAXParseException; 


public class MoreSwing extends JFrame {
    public MoreSwing(){
        initUI();
    }

    private int j;
    private String[] strArray = new String[4]; 
    JTextArea area = new JTextArea(strArray[1]);
    JButton nextButton = new JButton("Next");
    JButton prevButton = new JButton("Previous");
    JButton toggleAnswerButton = new JButton("Previous");
    boolean isToggledNext = false;
    boolean isToggledPrev = false;
    boolean dateLockBool=false;

    public final Date getDateTime(){
        Date date = new Date();
        return date;
    }

    //Loads everything from the selected week in the xml file into an array of strings via xml parsing.
    public final String[] xmlLoader(int s, String[] strArray_m){
        try {
            DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance();
            DocumentBuilder docBuilder = docBuilderFactory.newDocumentBuilder();
            Document doc = docBuilder.parse (new File("test.xml"));

            // normalize text representation
            doc.getDocumentElement ().normalize ();
            NodeList listOfWeek = doc.getElementsByTagName("week");

                Node firstWeekNode = listOfWeek.item(s-1);
                int totalWeeks = listOfWeek.getLength();
                strArray_m[3] = Integer.toString(totalWeeks);


                if(firstWeekNode.getNodeType() == Node.ELEMENT_NODE){
                    Element firstWeekElement = (Element)firstWeekNode;
                    //-------
                    NodeList dateList = firstWeekElement.getElementsByTagName("date");
                    Element dateElement = (Element)dateList.item(0);

                    NodeList textDateList = dateElement.getChildNodes();
                    strArray_m[0]= (((Node)textDateList.item(0)).getNodeValue().trim()).toString();

                    //-------
                    NodeList riddleList = firstWeekElement.getElementsByTagName("riddle");
                    Element riddleElement = (Element)riddleList.item(0);

                    NodeList textRiddleList = riddleElement.getChildNodes();
                    strArray_m[1]= (((Node)textRiddleList.item(0)).getNodeValue().trim()).toString();

                    //----
                    NodeList lWSList = firstWeekElement.getElementsByTagName("lastWeekSolution");
                    Element ageElement = (Element)lWSList.item(0);

                    NodeList textLWSList = ageElement.getChildNodes();
                    strArray_m[2]= (((Node)textLWSList.item(0)).getNodeValue().trim()).toString();

                    //------
                }//end of if clause

        }
            catch (SAXParseException err) {
        System.out.println ("** Parsing error" + ", line " 
             + err.getLineNumber () + ", uri " + err.getSystemId ());
        System.out.println(" " + err.getMessage ());

        }catch (SAXException e) {
        Exception x = e.getException ();
        ((x == null) ? e : x).printStackTrace ();

        }catch (Throwable t) {
        t.printStackTrace ();
        }
        return strArray_m;
    }

    //Handles all the data from the next button
    public final void doUpdate(int j, String[] strArray_m, JTextArea area){
        xmlLoader(j, strArray_m); //Load stuff from the xml file
        area.setText(strArray_m[1]); // update the text area
    }

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

    public final void dateLock(JButton nextButton){
            nextButton.setEnabled(!nextButton.isEnabled());
    }//End Datelock Method

    public final void toggleContent(JTextArea area, JButton toggleContent, String[] strArray_m){
        String toggle = toggleContent.getText();
        if(toggle.equalsIgnoreCase("Show Last Week's Answer")){
            toggleContent.setText("This Week's Brain Teaser");
            area.setText(strArray_m[2]);
        }//end if
        else{
            toggleContent.setText("Show Last Week's Answer");
            area.setText(strArray_m[1]);
        }//End else
    }//End ToggleContent method

    public final void initUI() {
        //add panel
        JPanel panel = new JPanel();
        getContentPane().add(panel);
        panel.setLayout(null);

        //Declare Scanner, input the week you want, load that week from XML file
        Scanner input = new Scanner(System.in);
        j = 1;       
        xmlLoader(j, strArray);
        isToggledPrev = toggleButton(prevButton, isToggledPrev);

        //Create text area with returned XML data as default text area text
        area.setBounds(50, 100, 350, 400);
        panel.add(area); // add text area to panel
        area.setText(strArray[1]);


        JLabel labelAuthor = new JLabel("Author: Damien Bell");
        labelAuthor.setBounds(620, 600, 150, 50);
        panel.add(labelAuthor);

        toggleAnswerButton.setBounds(95, 520, 250, 30);
        toggleAnswerButton.setText("Show Last Week's Answer");
        toggleAnswerButton.addActionListener(new ActionListener() {
           public void actionPerformed(ActionEvent ae){
             toggleContent(area, toggleAnswerButton, strArray);  
           }             
        });
        panel.add(toggleAnswerButton);

        nextButton.setBounds(550, 250, 80, 30);
        nextButton.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent ae){
            Date date1 = getDateTime();
            DateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH:mm:ss");
            try{
                Date date2 = dateFormat.parse(strArray[0]);
                if (date1.compareTo(date2) < 0 ){

                    dateLock(nextButton);
                    dateLockBool=true;
                }
            }   
            catch (Exception e) { System.err.println("Error: " + e.getMessage()); }
                if((j+2)> Integer.parseInt(strArray[3])){
                    isToggledNext = toggleButton(nextButton, isToggledNext);
                }
                if(isToggledPrev){
                   isToggledPrev = toggleButton(prevButton, isToggledPrev);
                }
               j++;
               doUpdate(j, strArray, area);
           } 
        });
        panel.add(nextButton);



        prevButton.setBounds(550, 350, 80, 30);
        prevButton.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent ae){
                if((j-2) == 0 ){
                   isToggledPrev = toggleButton(prevButton, isToggledPrev);
                }
                if(isToggledNext){
                   isToggledNext = toggleButton(nextButton, isToggledNext);
                }
                if(dateLockBool){
                   dateLockBool = false;
                   dateLock(nextButton);
                }
            j--;
            doUpdate(j, strArray, area);
          } 
        });
        panel.add(prevButton);


        //Add quit button
        JButton quitButton = new JButton("Quit");
        quitButton.setBounds(10,610,80,30);
        quitButton.addActionListener(new ActionListener() {
            public void actionPerformed (ActionEvent event){
                System.exit(0);
            }
        });

       panel.add(quitButton); // Add quit button to panel


       //Set overall window options for swing window
       setTitle("Quit Button Test");
       setSize(800,700);
       setLocationRelativeTo(null);
       setDefaultCloseOperation(EXIT_ON_CLOSE);
    }

    // Main, lol
    public static void main(String args[]){
        SwingUtilities.invokeLater(new Runnable() {
            public void run(){
                MoreSwing ms = new MoreSwing();
                ms.setVisible(true);
            }
        });
    }
}

Answer 1

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.

Answer 2

Given that you already appear to have an error in your XML file (because the solution presented in the first week appears to be the answer to itself, not lastWeekSolution as claimed), I'd recommend revising your structure. It's also going to cause problems with your edge cases. The first one logically can't have a solution element at all (because a blank solution is still a solution), and the last element can only have a solution element. It's also going to cause problems if you read it into something that ignores the order in the XML file - that is, treats it as set data.

I'd revise it to:

<riddle>
    <postingTimestamp>2012/01/23 00:00:00</postingTimestamp>
    <query>Why Did the chicken Cross the road?</query>
    <solution>Because I told him to!</solution>
</riddle>

I'd recommend making your XSD (you did define one, right?) allow between 0 and some multiple number of solutions (some riddles have no answer, some may have multiple). (Of course, I'd also probably store this in a database, not an XML file, but whatever).