Why this Simple Loop is Causing Problematic Behavior of my JFrame

Question

I am doing an assignment from the Java Exposure textbook, which was written in 2007. This book includes some code that I usually update to use some of the more recent features (just basic stuff). However, in this one I am running into a problem. All I tried to do is replace the show with setVisible(true) and change the Frame to a JFrame and add a gfx.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);. However, I noticed that this wouldn't actually cause the window to close. If I clicked many times, maybe 1/30 tries it would close. If I reduced the delay from 10 to 1, it usually closed within 2 tries. This of course led me to believe that the delay method is causing this erratic behavior. I tried Thread.sleep, but of course that didn't work. Is there any simply way to get this code so that the frame will close when I hit the close button? If there isn't, what would be the less simple way of doing it?

Here is the code:

// Lab30st.java
// The Screen Saver Program
// Student Version


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


public class Lab30st
{    
    public static void main(String args[])  
    {
        GfxApp gfx = new GfxApp();
        gfx.setSize(800,600);
        gfx.addWindowListener(new WindowAdapter() {public void
            windowClosing(WindowEvent e) {System.exit(0);}});
        gfx.show();
    }
}


class GfxApp extends Frame
{

    private int circleCount, circleSize;

    public GfxApp()
    {
        circleCount = 50;
        circleSize  = 30;
    }


    class Coord
    {
        private int xPos;
        private int yPos;

        public Coord(int x, int y) 
        {
            xPos = x;
            yPos = y;
        }
    }

    public void paint(Graphics g)
    {
        int incX = 5;
        int incY = 5;
        int diameter = 30;
        int timeDelay = 10;
        Circle c = new Circle(g,diameter,incX,incY,timeDelay);
        for (int k = 1; k <= 2000; k++)
        {   
            c.drawCircle(g);
            c.hitEdge();
        }

    } 
}



class Circle
{
    private int tlX;        // top-left X coordinate
    private int tlY;        // top-left Y coordinate
    private int incX;       // increment movement of X coordinate
    private int incY;       // increment movement of Y coordinate
    private boolean addX;   // flag to determine add/subtract of increment for X
    private boolean addY;   // flag to determine add/subtract of increment for Y
    private int size;       // diameter of the circle
    private int timeDelay;  // time delay until next circle is drawn



    public Circle(Graphics g, int s, int x, int y, int td)
    {
        incX = x;
        incY = y;
        size = s;
        addX = true;
        addY = false;
        tlX = 400;
        tlY = 300;
        timeDelay = td;
    }

    public void delay(int n)
    {
        long startDelay = System.currentTimeMillis();
        long endDelay = 0;
        while (endDelay - startDelay < n)
            endDelay = System.currentTimeMillis();  
    }

    public void drawCircle(Graphics g)
    {
        g.setColor(Color.blue);
        g.drawOval(tlX,tlY,size,size);
        delay(timeDelay);
        if (addX)
            tlX+=incX;
        else
            tlX-=incX;
        if (addY)
            tlY+=incY;
        else
            tlY-=incY;
    }


    public void newData()
    {
        incX = (int) Math.round(Math.random() * 7 + 5);
        incY = (int) Math.round(Math.random() * 7 + 5);
    }

    public void hitEdge()
    {
        boolean flag = false;
        if (tlX < incX)
        {
            addX = true;
            flag = true;
        }
        if (tlX > 800 - (30 + incX))  
        {
            addX = false;
            flag = true;
        }
        if (tlY < incY + 30)  // The +30 is due to the fact that the title bar covers the top 30 pixels of the window
        {
            addY = true;
            flag = true;
        }
        if (tlY > 600 - (30 + incY))  
        {
            addY = false;
            flag = true;
        }
        if (flag)
            newData();
    }

}

`gfx.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);` is correct. You're also correct that your "delay" is causeing the problem. Don't block the awt paint() thread, and don't implement delays by spinning the CPU. — BadZen, Nov 10 '16 at 01:00

score 2 · Accepted Answer · edited May 23 '17 at 11:53

You are "freezing" the Event Dispatch Thread with

public void delay(int n)
    {
        long startDelay = System.currentTimeMillis();
        long endDelay = 0;
        while (endDelay - startDelay < n)
            endDelay = System.currentTimeMillis();  
    }

This means that all the other stuff that is trying to happen (like closing the window) has to wait until the thread comes out of the "sleep". basically you shouldn't be doing the delay in the EDT, it should be on a different thread and then ask the EDT thread to update.

Your "busy wait" delay may cause other problems too. You can improve the behavior by using Thread.sleep()

See Java Event-Dispatching Thread explanation

Eli Algranti · Answer 2 · 2016-11-10T03:50:42.630

That's terrible.
You need to restructure the whole code.

Let's start with the really bad: delay is (almost) a busy wait, I haven't seen busy waits since BASIC was modern. It basically holds the CPU hostage to the thread, not only does it do nothing, no other thread (almost) can use the time slice. The reason I say almost is that calling the system time function causes a context switch that could allow other threads to run, but it is still bad.

The still pretty bad: Replacing with Thread.sleep. Better yes, no busy wait, but you are still holding the one and only UI thread. This means no other UI work can happen up to and including closing the main window.

What needs to happen:

Get an external timer (e.g. javax.swing.Timer) to trigger the draw event and do next part of the animation.

Search for "Java smooth animation" there are many examples of how to do this, double buffer and all.

Why this Simple Loop is Causing Problematic Behavior of my JFrame

2 Answers2