Issue when using Java Swing in a For-Loop

Question

I have created a simple for-loop that changes the amount of JTextFields and JLabels based on the value of a JSpinner, as seen in the following code:

        public static ArrayList<ArrayList<JTextField>> ChangeQuestionAnswerFields(int numberOfQuestions){
        ArrayList<ArrayList<JTextField>> txtFieldArray = new ArrayList<ArrayList<JTextField>>();

        JPanel scrollPanel = new JPanel(new GridBagLayout());
        //JScrollPane scrollPane = new JScrollPane(scrollPanel);
        frame.add(scrollPanel, BorderLayout.CENTER);
        GridBagConstraints c = new GridBagConstraints();
        c.gridx = 0;
        c.gridy = 0;


        for(int i = 0; i != numberOfQuestions; i++){
            JTextField tempQuestion = new JTextField(10);
            JTextField tempAnswer = new JTextField(10);
            JLabel tempQuestionHeader = new JLabel("Question " + (i + 1)+ " ");
            JLabel tempQuestionLbl = new JLabel("Question: ");
            JLabel tempAnswerLbl = new JLabel("Answer: ");

            ArrayList<JTextField> tempArrayList = new ArrayList<>();
            tempArrayList.add(tempQuestion);
            tempArrayList.add(tempAnswer);
            txtFieldArray.add(tempArrayList);

            c.gridy++;
            c.gridx = 0;
            scrollPanel.add(tempQuestionHeader, c);

            c.gridy++;
            c.gridx = 0;
            scrollPanel.add(tempQuestionLbl, c);

            c.gridx = 1;
            c.gridwidth = 3;
            scrollPanel.add(tempQuestion, c);

            c.gridy++;
            c.gridx = 0;
            scrollPanel.add(tempAnswerLbl, c);

            c.gridx = 1;
            c.gridwidth = 3;
            scrollPanel.add(tempAnswer, c);
        }
        return txtFieldArray;
    }
}

The value of the Spinner is passed into the method, and the method is called using a change listener (where noQuestions is the value of the JSpinner):

noQuestions.addChangeListener(e -> {
        ChangeQuestionAnswerFields((int) noQuestions.getValue());
        frame.revalidate();
        frame.repaint();
    });

This method is first called in the code when the screen first appears, and works properly. However, whenever the value of the spinner changes the original labels and fields stay on the screen and more text fields simply appear, or disappear on top.

https://i.stack.imgur.com/6d5jL.png - JSpinner has a value of 2 https://i.stack.imgur.com/PztVd.png - JSpinner has a value of 3

Is there any way to fix this? Any help is much appreciated.

Thanks, Tom

Minimal Runnable Example:

import javax.swing.*;
import java.awt.*;
import java.awt.event.WindowAdapter;
import java.awt.event.WindowEvent;
import java.util.ArrayList;

public class MainGUI {
    static JFrame frame = new JFrame("Math Reviser");

    public static void main(String[] args) {


        frame.setSize(400, 600);
        frame.setVisible(true);

        createScreen();
        frame.revalidate();
        frame.repaint();

    public static void createScreen(){
        frame.getContentPane().removeAll();

        JSpinner noQuestions = new JSpinner(new SpinnerNumberModel(1, 1, 10, 1));
        frame.add(noQuestions, BorderLayout.NORTH);
        );
        changeQuestionAnswerFields(1);

        frame.revalidate();
        frame.repaint();

        noQuestions.addChangeListener(e -> {
            changeQuestionAnswerFields((int) noQuestions.getValue());
            frame.revalidate();
            frame.repaint();
        });

    }

    public static ArrayList<ArrayList<JTextField>> changeQuestionAnswerFields(int numberOfQuestions){
        ArrayList<ArrayList<JTextField>> txtFieldArray = new ArrayList<ArrayList<JTextField>>();

        JPanel scrollPanel = new JPanel(new GridBagLayout());
        frame.add(scrollPanel, BorderLayout.CENTER);
        GridBagConstraints c = new GridBagConstraints();
        c.gridx = 0;
        c.gridy = 0;


        for(int i = 0; i != numberOfQuestions; i++){
            JTextField tempQuestion = new JTextField(10);
            JTextField tempAnswer = new JTextField(10);
            JLabel tempQuestionHeader = new JLabel("Question " + (i + 1)+ " ");
            JLabel tempQuestionLbl = new JLabel("Question: ");
            JLabel tempAnswerLbl = new JLabel("Answer: ");

            ArrayList<JTextField> tempArrayList = new ArrayList<>();
            tempArrayList.add(tempQuestion);
            tempArrayList.add(tempAnswer);
            txtFieldArray.add(tempArrayList);

            c.gridy++;
            c.gridx = 0;
            scrollPanel.add(tempQuestionHeader, c);

            c.gridy++;
            c.gridx = 0;
            scrollPanel.add(tempQuestionLbl, c);

            c.gridx = 1;
            c.gridwidth = 3;
            scrollPanel.add(tempQuestion, c);

            c.gridy++;
            c.gridx = 0;
            scrollPanel.add(tempAnswerLbl, c);

            c.gridx = 1;
            c.gridwidth = 3;
            scrollPanel.add(tempAnswer, c);
        }
        return txtFieldArray;
    }
}

Consider creating and posting a [minimal runnable example program](http://stackoverflow.com/help/mcve). Also, this method shouldn't be static. — Hovercraft Full Of Eels, Jul 18 '15 at 23:29
@HovercraftFullOfEels Would the JVM be smart enough to know that the ArrayList should actually be instantiated outside the loop, and do so accordingly? — Evan Bechtol, Jul 18 '15 at 23:34
@HovercraftFullOfEels how can i reference changeQuestionAnswerFields (non-static method) from PSVM (static)? — Thomas Williams, Jul 18 '15 at 23:41
@ThomasWilliams By creating an instance of the class which contains that method and calling the method on it. — Tom, Jul 18 '15 at 23:43
I changed the class to non-static, but this didn't fix the issue — Thomas Williams, Jul 18 '15 at 23:48
I left the frame at its defult size, and increased the value of the spinner, i then increased the size of the frame and got the following result. http://i.imgur.com/HPgnmPg.png — Thomas Williams, Jul 18 '15 at 23:49
I fixed the issue. Before the for-loop i removed all components from the panel. — Thomas Williams, Jul 18 '15 at 23:55
That's a kludge not a fix. Again, use a CardLayout as suggested earlier. — Hovercraft Full Of Eels, Jul 19 '15 at 02:09

score 2 · Answer 1 · answered Jul 19 '15 at 00:20

Using static variables and methods is an indication of a poorly designed application. There is no need for the static variables or methods. I suggest you read the section from the Swing tutorial on How to Use Labels. The LabelDemo.java code will show you how to create a panel containing all the components. This panel will then be added to the frame. This panel will also contain all the instance variables you need for your program.

Not only that the example will show you how to create the GUI components on the EDT which is something you should always do to prevent random errors since Swing was designed to be single threaded.

However, the main problem with your existing code is that you continue to create and add new panels to the content pane of the frame. Try changing the spinner to 2 and then resize the frame. Then try changing the spinner to 3 and resize the frame. After the resizing the first panel is displayed. This is because Swing will paint the last component added first so the first panel added will be painted on top of the last panel you created.

You can change this in your existing code by removing the old panel before adding the new panel:

static JFrame frame = new JFrame("Math Reviser");
static JPanel scrollPanel = new JPanel();
...
frame.remove(scrollPanel);
//JPanel scrollPanel = new JPanel(new GridBagLayout());
scrollPanel = new JPanel(new GridBagLayout());

However, I do not recommend this approach. As I initially suggestion you need to redesign the entire class. When you do the redesign I would use a BorderLayout on your panel and then you can add your spinner to the PAGE_START and then add a JScrollPane to the CENTER of the panel.

Then when you want to create a new panel you add the panel to the scrollpane using code like:

scrollPane.setViewportView( scrollPanel );

The scrollpane will refresh itself and you don't need to worry about revalidate() or repaint() or anything else.

Issue when using Java Swing in a For-Loop

1 Answers1