Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.java.programmer > #30394

Re: Could this be done better

Show key headers only | View raw

Op Monday 24 Nov 2014 15:23 CET schreef Eric Sosman:

> On 11/24/2014 6:16 AM, Cecil Westerhof wrote:
>> I have mostly programmed back-ends. But I now want also to do some
>> front-end programming.
>>
>> I wrote the following simple program. It does nothing useful, it
>> just displays a description in different colours with the colour
>> appended to the description.
>>
>> I was just wondering if this could be done better. Especially
>> ‘updateColour’. First I did not removed the text, but then I could
>> see the colour changing before the text was changed. This way I
>> find more pleasing and I do not notice that the text disappears.
>
> One rather important point you've overlooked: Swing is (mostly)
> not thread-safe.  Swing methods should only be invoked on the Event
> Dispatching Thread (EDT), except for the very few methods that are
> specifically documented as safe for use from other threads.
>
> Your program violates this rule in two ways:
>
> - The ColourCycling constructor builds a JFrame and invokes
> assorted methods on it, and also builds a JLabel -- but the
> constructor is called from a thread that is not the EDT.
>
> - The updateColour() method manipulates the JLabel, but is
> called from some random thread created by Timer -- again, not
> the EDT.
>
> The first problem is one of those "it almost always works
> right" issues, the kind that will not bite you -- until the demo
> for the Big Boss and the Big Customer.  (Sun themselves did not
> at first realize it was a problem, and many early Swing examples
> illustrated exactly the same mistake you've made.)  Fortunately,
> it's easy to fix: Use the SwingUtilities.invokeLater() method to
> run the ColourCycling constructor on the EDT instead of on the
> initial thread.  (For clarity's sake, I'd suggest moving the
> JLabel construction inside the ColourCycling constructor, too:
> That's where the byte code winds up, but let's be explicit, OK?)

Done.


> The second problem is (most likely) the cause of the strange
> display glitches you refer to: You're updating the display from
> a non-EDT thread, and the EDT doesn't "know" right away that things
> have changed.  There are at least two ways to cure it:
>
> - Inside the TimerTask, use SwingUtilities.invokeLater() to
> launch a Runnable that calls updateColour(), so that the call runs
> on the EDT instead of on the Timer's thread.

I choose for this solution: was the easiest to do. ;-)


> - Use javax.swing.Timer instead of java.util.Timer.  The
> mechanics are a bit different -- you write an ActionListener
> instead of subclassing TimerTask -- but the action listener
> automatically runs on the EDT, where it's safe to manipulate
> your GUI.

Or is there a reason why did would be better?


It looks OK now. The code has become:
    import java.awt.*;
    import java.util.*;

    import javax.swing.*;

    @SuppressWarnings("serial")
    public class ColourCycling extends JFrame {
        // public functions
        public ColourCycling() {
            java.util.Timer colourTimer = new java.util.Timer();

            getContentPane().setBackground(background);
            setLayout(new GridBagLayout());
            label = new JLabel(description + " : " + "XXXXXXXXXXXXXXX");
            label.setFont(new Font("serif", Font.BOLD, 25));
            add(label);
            pack();
            setSize(getWidth(), 3 * getHeight());
            label.setBackground(background);
            updateColour(getCurrentColour());
            colourTimer.schedule(new SwitchColour(), delay, delay);
            setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
            setTitle(description);
            setVisible(true);
        }

        public static void main(String[] args) {
            SwingUtilities.invokeLater(new Runnable() {
                    public void run() {
                        new ColourCycling();
                    }
                });
        }

        // private classes
        private class Colour {
            // public
            public Colour(Color color, String description) {
                this.color          = color;
                this.description    = description;
            }

            public Color getColor() {
                return color;
            }

            public String getDescription() {
                return description;
            }

            // private
            private Color   color;
            private String  description;
        }

        private class SwitchColour extends TimerTask {
            public void run() {
            SwingUtilities.invokeLater(new Runnable() {
                    public void run() {
                        updateColour(getNextColour());
                    }
                });
            }
        }

        //private functions
        private Colour getCurrentColour() {
            return colours[currentColour];
        }

        private Colour getNextColour() {
            currentColour = (currentColour + 1) % colours.length;
            return getCurrentColour();
        }

        private void updateColour(Colour newColour) {
            label.setForeground(newColour.getColor());
            label.setText(description + " : " + newColour.getDescription());
        }

        // private variables
        private Color       background      = Color.lightGray;
        private Colour[]    colours         = new Colour[] {
            new Colour(Color.blue,                  "Blue"),
            new Colour(Color.red,                   "Red"),
            new Colour(Color.orange,                "Orange"),
            new Colour(Color.yellow,                "Yellow"),
            new Colour(Color.green,                 "Green"),
            new Colour(Color.cyan,                  "Cyan"),
            new Colour(Color.magenta,               "Magenta"),
            new Colour(new Color(0x9C, 0x5D, 0x52), "Brown"),
        };
        private int         currentColour   = 0;
        private int         delay           = 4000;
        private String      description     = "JTextArea Color Change Example";
        private JLabel      label;

    }

By the way, the reason I use:
    @SuppressWarnings("serial")
is that I compile with:
    javac -Xlint ColourCycling.java

Well, I need to make a more interesting program.


> Recommended reading:
>
> http://docs.oracle.com/javase/tutorial/uiswing/concurrency/index.html

My reading list just became a little bigger again. :-D

--
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

Back to comp.lang.java.programmer | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Could this be done better Cecil Westerhof <Cecil@decebal.nl> - 2014-11-24 12:16 +0100
  Re: Could this be done better "Qu0ll" <Qu0llSixFour@gmail.com> - 2014-11-24 22:49 +1100
    Re: Could this be done better Cecil Westerhof <Cecil@decebal.nl> - 2014-11-24 17:54 +0100
      Re: Could this be done better Knute Johnson <eternal@knutejohnson.com> - 2014-11-24 09:37 -0800
        Re: Could this be done better Cecil Westerhof <Cecil@decebal.nl> - 2014-11-24 19:27 +0100
  Re: Could this be done better Eric Sosman <esosman@comcast-dot-net.invalid> - 2014-11-24 09:23 -0500
    Re: Could this be done better Cecil Westerhof <Cecil@decebal.nl> - 2014-11-24 17:45 +0100
      Re: Could this be done better Eric Sosman <esosman@comcast-dot-net.invalid> - 2014-11-24 13:42 -0500
        Re: Could this be done better Cecil Westerhof <Cecil@decebal.nl> - 2014-11-25 00:12 +0100
    Re: Could this be done better Knute Johnson <eternal@knutejohnson.com> - 2014-11-24 09:19 -0800
      Re: Could this be done better Knute Johnson <eternal@knutejohnson.com> - 2014-11-24 09:41 -0800
        Re: Could this be done better Cecil Westerhof <Cecil@decebal.nl> - 2014-11-25 00:55 +0100
      Re: Could this be done better Cecil Westerhof <Cecil@decebal.nl> - 2014-11-25 00:11 +0100
        Re: Could this be done better Knute Johnson <eternal@knutejohnson.com> - 2014-11-24 16:26 -0800
  Re: Could this be done better Brixomatic <brixomatic@yahoo.com> - 2014-11-25 19:58 +0100
    Re: Could this be done better Eric Sosman <esosman@comcast-dot-net.invalid> - 2014-11-25 14:36 -0500
      Re: Could this be done better Brixomatic <brixomatic@yahoo.com> - 2014-11-26 01:58 +0100
        Re: Could this be done better Eric Sosman <esosman@comcast-dot-net.invalid> - 2014-11-26 08:48 -0500
          Re: Could this be done better Brixomatic <brixomatic@yahoo.com> - 2014-11-29 19:43 +0100
            Re: Could this be done better Eric Sosman <esosman@comcast-dot-net.invalid> - 2014-11-29 15:44 -0500
              Re: Could this be done better Eric Sosman <esosman@comcast-dot-net.invalid> - 2014-11-29 15:47 -0500

csiph-web