Counter GUI in Swing, updating the count in the background I made a simple counter GUI for practice. The user presses a button to start the counter. The button click executes a SwingWorker task that updates a JLabel with the count. The update occurs in the doInBackground method. 

Is this a bad idea? Even for something as simple as this, I should still update the component from a method that runs on the event dispatch thread, such as process?

If so, why would it be a bad practice for something as simple as this?

Main.java: 

package counter;

import javax.swing.SwingUtilities;

public class Main {
    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            public void run() {
                CounterView view = new CounterView();
                view.activate();
            }
        });
    }
}


ProgressBar.java:

package counter;

import java.awt.BorderLayout;
import javax.swing.*;
import javax.swing.JFrame;
import javax.swing.JButton;
import javax.swing.JLabel;
import javax.swing.SwingWorker;
import javax.swing.SwingConstants;
import java.awt.event.*;

public class CounterView {
    private JFrame frame;
    private JPanel gui;
    private JButton button;
    private JLabel count;

    public CounterView() {
        customizeFrame();
        createMainPanel();
        createCountText();
        createButton();
        addComponentsToFrame();
    }

    private void customizeFrame() {
        frame = new JFrame();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    }

    private void createMainPanel() {
        gui = new JPanel();
        gui.setLayout(new BorderLayout());
    }

    private void createCountText() {
        count = new JLabel("0");
        count.setHorizontalAlignment(SwingConstants.CENTER);
    }

    private void createButton()  {
        button = new JButton("Start counter");
        button.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                // Create the worker
                SwingWorker&lt;Void, Void&gt; worker = new SwingWorker&lt;Void, Void&gt;() {

                    @Override
                    protected Void doInBackground() throws Exception {
                        for (int i = 0; i &lt;= 50; i++) {
                            Thread.sleep(100);
                            System.out.println(i);
                            text.setText(Integer.toString(i));
                        }

                        return null;
                    }
                };

                // Start the worker
                worker.execute();
            }
        });
    }

    private void addComponentsToFrame() {
        gui.add(count, BorderLayout.CENTER);
        gui.add(button, BorderLayout.SOUTH);
        frame.add(gui);
        frame.pack();
    }

    public void activate() {
        frame.setVisible(true);
    }
}
To answer your question:


  Q. Is this a bad idea?  


A. No, it's fine in your case, but creating a new Thread is easier, like this:

private void createButton() {
    button = new JButton("Start counter");
    button.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            button.setEnabled(false); // can't click while counter is running
            Thread background = new Thread(new Runnable() {
                @Override
                public void run() {
                    for (int i = 0; i &lt;= 50; i++) {
                        try {
                            Thread.sleep(100);
                        } catch(InterruptedException e) {
                            e.printStackTrace();
                        }
                        System.out.println(i);
                        count.setText(Integer.toString(i));
                    }
                    button.setEnabled(true); //click-able after the counter ends
                }
            });
            background.start();
        }
    });
}




There are some things you can fix in your code to improve it:


Use of throws Exception in your doInBackGround method. throwing Exception is not good to use because it is vague, and unnecessary in your case. Just throw InterruptedException instead.

@Override
protected Void doInBackground() throws InterruptedException {
    //...
}

Error. This line: text.setText(Integer.toString(i)); throws an Error: "text cannot be resolved." I believe you meant to do: count.setText(Integer.toString(i));
activate() method. Make your life easier by cutting down a few lines: Just set the frame visible in your constructor, like this:

public CounterView() {
    customizeFrame();
    createMainPanel();
    createCountText();
    createButton();
    addComponentsToFrame();
    frame.setVisible(true);
}


This removes the need for your activate method completely.
Notice that if you click the button twice, then 2 counters start? I don't think you want that. disable the button after it is clicked to avoid this. Example:

private void createButton() {
    button = new JButton("Start counter");
    button.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            button.setEnabled(false); //added this line
            //...





Other than those, your code looks good. good indentation, variable names, method names, etc.