frame.repaint() not working

Question

This is a source I got directly from "Head First Java" but whatever I do I can't seem to make it work, and I don't know what I may be missing

import javax.swing.*;

import java.awt.*;

import java.awt.event.*;

public class SimpleGui3C implements ActionListener {

    JFrame frame;

    public static void main(String[] args) {

        SimpleGui3C gui = new SimpleGui3C();
        gui.go();
    }

public void go() {

    MyDrawPanel drawPanel = new MyDrawPanel();

    JFrame frame = new JFrame();
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

    JButton button = new JButton("Change colors");
    button.addActionListener(this);

    frame.getContentPane().add(BorderLayout.SOUTH, button);
    frame.getContentPane().add(BorderLayout.CENTER, drawPanel);
    frame.setSize(300, 300);
    frame.setVisible(true);
}

public void actionPerformed(ActionEvent event) {


    frame.repaint();
    }
}

class MyDrawPanel extends JPanel {

    public void paintComponent(Graphics g) {

        g.setColor(Color.white);
        g.fillRect(0, 0, 300, 300);

    int red = (int) (Math.random() * 255);
    int green = (int) (Math.random() * 255);
    int blue = (int) (Math.random() * 255);

    Color randomColor = new Color(red, green, blue);
    g.setColor(randomColor);
    g.fillOval(70, 70, 100, 100);

}
}

I tried to find another way to do it which doesn't involve repaint but rather createing a new instance of MyDrawPanel whenever the event occurs, but it still doesn't work since I didn't found a way to clear the panel properly, the only hack I've found so far is to do this, but it's not what I want to acheive ...

import javax.swing.*;

import java.awt.*;

import java.awt.event.*;

public class SimpleGui3C implements ActionListener {

    JFrame frame;

    public static void main(String[] args) {

    SimpleGui3C gui = new SimpleGui3C();
    gui.go();
}

public void go() {

    MyDrawPanel drawPanel = new MyDrawPanel();

    JFrame frame = new JFrame();
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

    JButton button = new JButton("Change colors");
    button.addActionListener(this);

    frame.getContentPane().add(BorderLayout.SOUTH, button);
    frame.getContentPane().add(BorderLayout.CENTER, drawPanel);
    frame.setSize(300, 300);
    frame.setVisible(true);
}

    public void actionPerformed(ActionEvent event) {
        go();
    }
}



class MyDrawPanel extends JPanel {

    int red;
    int green;
    int blue;

    public MyDrawPanel() {

    this.red = (int) (Math.random() * 255);
    this.green = (int) (Math.random() * 255);
    this.blue = (int) (Math.random() * 255);
}

public void paintComponent(Graphics g) {

    g.setColor(Color.white);
    g.fillRect(0, 0, 300, 300);

    Color randomColor = new Color(this.red, this.green, this.blue);
    g.setColor(randomColor);
    g.fillOval(70, 70, 100, 100);

}
}

Answer 1

While dealing with Swing, one should keep some thingies in mind. As I looked at the code used by you, I felt like pointing it out to you, to be on the right track.

1. Swing based applications are started from their own respective thread, called the EventDispatcherThread ( EDT ) and not directly from main. More information, on the topic can be found on Concurrency in Swing
2. Try not to perform any calculations inside the paintComponent ( ... ) method, instead perform these calculations somewhere else, and simply call repaint ()
3. The access specifier of paintComponent ( ... ) is protected and not public, hence while overriding methods of super class, try not to change the methods access as much as possible, until not necessary.
4. When you are drawing on a JPanel, simply call the repaint () on the instance of the JPanel, instead of the top level container's
5. When one is extending any JComponenet/JPanel, always try to override, the said JComponent/JPanel's getPreferredSize (), as many a layouts will return 0, if none is specified, hence no painting will be done.
6. Do remember to call super.paintComponent ( g ), as the first line inside paintComponent ( ... ). Added a comment for more clarity.
7. Instead of setting size on JFrame, try to call pack (), as stated in Java Docs for the benefits attached. The pack method sizes the frame so that all its contents are at or above their preferred sizes. An alternative to pack is to establish a frame size explicitly by calling setSize or setBounds (which also sets the frame location). In general, using pack is preferable to calling setSize, since pack leaves the frame layout manager in charge of the frame size, and layout managers are good at adjusting to platform dependencies and other factors that affect component size.

Here is the modified code ( just added a method inside DrawPanel, called setValues (), where calculations are done and repaint () is called ), based on above points:

import java.awt.*;
import java.awt.event.*;
import javax.swing.*;

public class SimpleGui implements ActionListener {

    private MyDrawPanel drawPanel;

    public static void main(String[] args) {
        Runnable r = new Runnable () {
            @Override
            public void run () {
                new SimpleGui ().go ();
            }
        };
        EventQueue.invokeLater ( r );
    }

    public void go() {
        drawPanel = new MyDrawPanel();

        JFrame frame = new JFrame();
        frame.setDefaultCloseOperation( JFrame.DISPOSE_ON_CLOSE );
        JButton button = new JButton( "Change colors" );
        button.addActionListener( this );

        frame.add( drawPanel, BorderLayout.CENTER );
        frame.add( button, BorderLayout.PAGE_END );
        frame.pack ();
        frame.setLocationByPlatform ( true );
        frame.setVisible(true);
    }

    public void actionPerformed(ActionEvent event) {
        drawPanel.setValues ();
    }
}

class MyDrawPanel extends JPanel {

    private int width = 300;
    private int height = 300;

    private int red;
    private int green;
    private int blue;

    private Color randomColor;

    /*
     * Make this one customary habbit,
     * of overriding this method, when
     * you extends a JPanel/JComponent,
     * to define it's Preferred Size.
     * Now in this case we want it to be 
     * as big as the Image itself.
     */
    @Override
    public Dimension getPreferredSize () {
        return new Dimension ( width, height );
    }

    public void setValues () {
        red = ( int ) ( Math.random() * 255 );
        green = ( int) ( Math.random() * 255 );
        blue = ( int ) ( Math.random() * 255 );

        randomColor = new Color( red, green, blue );

        repaint ();
    }

    /*
     * This is where the actual Painting
     * Code for the JPanel/JComponent goes.
     * Here the first line super.paintComponent(...),
     * means we want the JPanel to be drawn the usual 
     * Java way first (this usually depends on the opaque
     * property of the said JComponent, if it's true, then
     * it becomes the responsibility on the part of the
     * programmer to fill the content area with a fully
     * opaque color. If it is false, then the programmer
     * is free to leave it untouched. So in order to 
     * overcome the hassle assoicated with this contract,
     * super.paintComponent(g) is used, since it adheres
     * to the rules, and performs the same task, depending
     * upon whether the opaque property is true or false),
     * then later on we will add our image to it, by 
     * writing the other line, g.drawImage(...).
     */
    @Override
    protected void paintComponent(Graphics g) {
        super.paintComponent ( g );

        g.setColor(randomColor);
        g.fillOval(70, 70, 100, 100);
    }
}

Answer 2

This will do what you want....

import javax.swing.*;
import java.awt.*;
import java.awt.event.*;

public class SimpleGui3C implements ActionListener {

    static JFrame frame = null;  // changed here...

    public static void main(String[] args) {
        frame = new JFrame();  // changed here....
        SimpleGui3C gui = new SimpleGui3C();
        gui.go();
    }

    public void go() {
        MyDrawPanel drawPanel = new MyDrawPanel();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        JButton button = new JButton("Change colors");
        button.addActionListener(this);
        frame.getContentPane().add(BorderLayout.SOUTH, button);
        frame.getContentPane().add(BorderLayout.CENTER, drawPanel);
        frame.setSize(300, 300);
        frame.setVisible(true);
    }

    public void actionPerformed(ActionEvent event) {
        frame.repaint();
    }
}

class MyDrawPanel extends JPanel {

    public void paintComponent(Graphics g) {
        g.setColor(Color.white);
        g.fillRect(0, 0, 300, 300);
        int red = (int) (Math.random() * 255);
        int green = (int) (Math.random() * 255);
        int blue = (int) (Math.random() * 255);
        Color randomColor = new Color(red, green, blue);
        g.setColor(randomColor);
        g.fillOval(70, 70, 100, 100);
    }
}

In your go() method... Everytime you are creating a new object by JFrame frame = new JFrame(); and hence on your every click a new frame appears on the screen...

By creating the object in main() method we are only calling the repaint() method everytime on the same jframe object and not creating a new object....

Hoping it helped...