Path: csiph.com!usenet.pasdenom.info!weretis.net!feeder1.news.weretis.net!feeder.erje.net!eu.feeder.erje.net!newsfeed.tele2net.at!news.panservice.it!feed.xsnews.nl!border02.ams.xsnews.nl!feeder04.ams.xsnews.nl!feeder03.ams.xsnews.nl!frontend-F09-15.ams.news.kpn.nl From: Cecil Westerhof Newsgroups: comp.lang.java.programmer Subject: Re: Could this be done better Organization: Decebal Computing References: <87zjbg97c5.fsf@Equus.decebal.nl> X-Face: "(y8cC@tg_12{">GF'UXTW]FHI2wMiZNrnf'1EFQ&O#$m:f#O7+7}kR,v+Pti8=Vi/Z"g^?b"E X-Homepage: http://www.decebal.nl/ Date: Mon, 24 Nov 2014 17:45:53 +0100 Message-ID: <87ppcc8s3i.fsf@Equus.decebal.nl> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) Cancel-Lock: sha1:PGSQauzqSfQY+QJq+cz0yURisnE= MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Lines: 183 NNTP-Posting-Host: 77.171.42.144 X-Trace: 1416848337 news.kpn.nl 21177 77.171.42.144@kpn/77.171.42.144:52363 Xref: csiph.com comp.lang.java.programmer:30394 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