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

Groups > comp.lang.java.programmer > #30390 > unrolled thread

Could this be done better

Back to article view | Back to comp.lang.java.programmer

Contents

  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

Page 1 of 2  [1] 2  Next page →

#30390 — Could this be done better

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.

    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.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) {
            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() {
                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.setText("");
            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           = new JLabel(description + " : " + "XXXXXXXXXXXXXXX");

    }

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

[toc] | [next] | [standalone]

#30391

"Cecil Westerhof"  wrote in message news:87zjbg97c5.fsf@Equus.decebal.nl...

> 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.

Yes - use JavaFX instead of Swing.

JavaFX is the *official* GUI toolkit for use with Java applications as 
stated by Oracle themselves.  Swing is deprecated (though not in the pure 
Java sense) in that it is not getting any love (updates, bug fixes, new 
features) and never will.

Java developers should only use Swing if they have to modify an existing 
application or write a new one that works with existing frameworks that 
happen to be Swing-based.

Otherwise, use JavaFX.  Don't look back.

--
And loving it,

-Qu0ll (Rare, not extinct)
_________________________________________________
Qu0llSixFour@gmail.com
[Replace the "SixFour" with numbers to email me] 

[toc] | [prev] | [next] | [standalone]

#30393

Op Monday 24 Nov 2014 12:49 CET schreef Qu0ll:

> "Cecil Westerhof" wrote in message
> news:87zjbg97c5.fsf@Equus.decebal.nl...
>
>> 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.
>
> Yes - use JavaFX instead of Swing.
>
> JavaFX is the *official* GUI toolkit for use with Java applications
> as stated by Oracle themselves. Swing is deprecated (though not in
> the pure Java sense) in that it is not getting any love (updates,
> bug fixes, new features) and never will.
>
> Java developers should only use Swing if they have to modify an
> existing application or write a new one that works with existing
> frameworks that happen to be Swing-based.
>
> Otherwise, use JavaFX.  Don't look back.

Well there where several reasons to use Swing, but most seem not valid
anymore.
- I work mostly with Linux. JavaFX was not supported there, but it
looks like that is solved.
- I also work with Clojure. The ‘only’ GUI to use was Swing, but it
seems that JavaFX should be doable also.
- It also looks like JavaFX is supported on Android.
- But this one is still valid I am afraid. At a lot of projects they
expect you to have Swing experience. JavaFX rarely as far as I have
seen. So I think it is good to start with Swing, but not forget about
JavaFX.

By the way: is JavaFX thread safe? Swing is not I found out.

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

[toc] | [prev] | [next] | [standalone]

#30396

On 11/24/2014 08:54, Cecil Westerhof wrote:
> By the way: is JavaFX thread safe? Swing is not I found out.

No.  Same issues I'm afraid.

-- 

Knute Johnson

[toc] | [prev] | [next] | [standalone]

#30398

Op Monday 24 Nov 2014 18:37 CET schreef Knute Johnson:

> On 11/24/2014 08:54, Cecil Westerhof wrote:
>> By the way: is JavaFX thread safe? Swing is not I found out.
>
> No.  Same issues I'm afraid.

Well this time I am warned. ;-)

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

[toc] | [prev] | [next] | [standalone]

#30392

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?)

     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.

     - 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.

     Recommended reading:

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

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

#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

[toc] | [prev] | [next] | [standalone]

#30399

On 11/24/2014 11:45 AM, Cecil Westerhof wrote:
> Op Monday 24 Nov 2014 15:23 CET schreef Eric Sosman:
>> [...]
>> 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?

     (Shrug.)  For your purpose I don't see much difference.
But suppose you wanted to add a few more buttons to your GUI:

	"Next colour"
	"Previous colour"
	"Start cycling"
	"Stop cycling"

You'll need an ActionListener for the "Next colour" button,
and with javax.swing.Timer you could re-use that exact same
ActionListener for cycling.  Essentially, the Timer would fire
the same kind of events the button does, and they'd be handled
by the same chunk of code.

     The JavaDoc for javax.swing.Timer mentions another possible
advantage: If you have a *lot* of timed activity in your GUI,
you might wind up creating a *lot* of java.util.Timer threads if
you weren't careful.  With javax.swing.Timer that's not an issue;
javax.swing.Timer "multiplexes" all the different events on a
single internal thread.  FWIW, I myself have never created so
many Timers (of either kind) that thread multiplicity ever began
to become a problem, but, hey ...

     An even more flexible and feature-ridden alternative would be
to use java.util.concurrent.ScheduledThreadPoolExecutor.

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

#30402

Op Monday 24 Nov 2014 19:42 CET schreef Eric Sosman:

>>> - 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?
>
> (Shrug.)  For your purpose I don't see much difference.
> But suppose you wanted to add a few more buttons to your GUI:
>
> 	"Next colour"
> 	"Previous colour"
> 	"Start cycling"
> 	"Stop cycling"
>
> You'll need an ActionListener for the "Next colour" button,
> and with javax.swing.Timer you could re-use that exact same
> ActionListener for cycling.  Essentially, the Timer would fire
> the same kind of events the button does, and they'd be handled
> by the same chunk of code.
>
> The JavaDoc for javax.swing.Timer mentions another possible
> advantage: If you have a *lot* of timed activity in your GUI,
> you might wind up creating a *lot* of java.util.Timer threads if
> you weren't careful.  With javax.swing.Timer that's not an issue;
> javax.swing.Timer "multiplexes" all the different events on a
> single internal thread.  FWIW, I myself have never created so
> many Timers (of either kind) that thread multiplicity ever began
> to become a problem, but, hey ...
>
> An even more flexible and feature-ridden alternative would be
> to use java.util.concurrent.ScheduledThreadPoolExecutor.

Looks like I have some experimenting to do. :-D

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

[toc] | [prev] | [next] | [standalone]

#30395

On 11/24/2014 06:23, Eric Sosman wrote:
> 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?)
>
>      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.
>
>      - 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.
>
>      Recommended reading:
>
> http://docs.oracle.com/javase/tutorial/uiswing/concurrency/index.html
>


Eric gave you all the biggies, I'll nit-pick a little.

 From the docs, JFrame:

"As a convenience, the add, remove, and setLayout methods of this class 
are overridden, so that they delegate calls to the corresponding methods 
of the ContentPane. For example, you can add a child component to a 
frame as follows:

        frame.add(child);"

so you can skip the call to getContentPane().

You first call pack() and the call setSize() on your frame.  Only one is 
really necessary.  It is often easier to use your content to control the 
size of your frame.  In this case making the JLabel much larger, adding 
a border and then calling pack().  You can also set the preferredSize of 
the frame or a component and call pack();  Getting labels and other 
components to be the right size can be tricky.  Using GridBagLayout as 
you did, causes the component to be sized to its preferredSize and then 
put in the middle of the frame.  Using BorderLayout, the default, will 
cause the content to fill the area of the frame that is available.

Very small nit: The frame title is a parameter of the constructor.  You 
could have used:

super(description)  as the first line.

In updateColour() you call label.setText(""), then change the color and 
then set the text again.  What you were seeing with the text changing 
and then the color changing were very likely artifacts from updating the 
JLabel from other than the event dispatch thread.

Another very small nit: you used the older versions of color constants 
with the lower case field names.  In 1.4 new constants were added with 
upper case names to match current convention.

Here is a JLabel that displays the current time.  Looks very similar to 
your example.  I added an empty border to give you an example of how to 
do that too.

//package com.knutejohnson.components;

import java.awt.*;
import java.awt.event.*;
import java.text.*;
import java.util.*;
import javax.swing.*;
import javax.swing.border.*;

/**
  * JTimeLabel is a JLabel that displays the current time.
  *
  * @author Knute Johnson
  */
public class JTimeLabel extends JLabel implements ActionListener {
     private final SimpleDateFormat sdf;
     private final javax.swing.Timer timer = new 
javax.swing.Timer(250,this);

     /**
      * Constructs a JTimeLabel
      *
      * @param   text    The initial text.
      * @param   horizontalAlignment     One of the following; LEFT, CENTER,
      *                  RIGHT, LEADING or TRAILING.
      * @param   pattern The time display pattern.
      *
      * @see     JLabel
      * @see     SimpleDateFormat
      */
     public JTimeLabel(String text, int horizontalAlignment, String 
pattern) {
         super(text, horizontalAlignment);
         sdf = new SimpleDateFormat(pattern);
     }

     /**
      * Constructs a JTimeLabel with a format of {@code HH:mm:ss}.
      */
     public JTimeLabel() {
         this("00:00:00",JLabel.CENTER,"HH:mm:ss");
     }

     /**
      * Constructs a JTimeLabel with the specified initial text and a format
      * of {@code HH:mm:ss}.
      *
      * @param   text    The initial text.
      */
     public JTimeLabel(String text) {
         this(text,JLabel.CENTER,"HH:mm:ss");
     }

     /**
      * Constructs a JTimeLabel with the specified intitial text and 
horizontal
      * alignment.
      *
      * @param   text    The initial text.
      * @param   horizontalAlignment     One of the following; LEFT, CENTER,
      *                  RIGHT, LEADING or TRAILING.
      */
     public JTimeLabel(String text, int horizontalAlignment) {
         this(text,horizontalAlignment,"HH:mm:ss");
     }

     /**
      * Starts the clock running
      */
     public void start() {
         timer.start();
     }

     /**
      * Stops the clock
      */
     public void stop() {
         timer.stop();
     }

     public void actionPerformed(ActionEvent ae) {
         setText(sdf.format(new Date()));
     }

     /**
      * For testing only
      *
      * @param args the command line arguments, unused
      */
     public static void main(String[] args) {
         EventQueue.invokeLater(new Runnable() {
             public void run() {
                 JFrame f = new JFrame();
                 f.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                 JTimeLabel tl = new JTimeLabel();
                 tl.setBorder(new EmptyBorder(40,40,40,40));
                 tl.setFont(new Font("Arial",Font.BOLD,32));
                 f.add(tl);
                 tl.start();
                 f.pack();
                 f.setVisible(true);
             }
         });
     }
}


-- 

Knute Johnson

[toc] | [prev] | [next] | [standalone]

#30397

         // 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() {
                 updateColour(getNextColour());
             }
         }

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

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

As much as I like your class Colour, it is probably easier and simpler 
to use an enum in this case.

import java.awt.*;

public class test {
     enum Colours {
         Red(Color.RED),Blue(Color.BLUE),Green(Color.GREEN);

         private final Color value;

         Colours(Color value) {
             this.value = value;
         }

         public Color value() {
             return value;
         }
     }

     public static void main(String... args) {
         System.out.println(Colours.Red);
         System.out.println(Colours.Red.value());

         System.out.println();

         for (Colours colour : Colours.values())
             System.out.println(colour+":"+colour.value());
     }
}

C:\Users\Knute Johnson>java test
Red
java.awt.Color[r=255,g=0,b=0]

Red:java.awt.Color[r=255,g=0,b=0]
Blue:java.awt.Color[r=0,g=0,b=255]
Green:java.awt.Color[r=0,g=255,b=0]

-- 

Knute Johnson

-- 

Knute Johnson

[toc] | [prev] | [next] | [standalone]

#30405

Op Monday 24 Nov 2014 18:41 CET schreef Knute Johnson:

>
> // 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() {
> updateColour(getNextColour());
> }
> }
>
> //private functions
> private Colour getCurrentColour() {
> return colours[currentColour];
> }
>
> private Colour getNextColour() {
> currentColour = (currentColour + 1) % colours.length;
> return getCurrentColour();
> }
>
> As much as I like your class Colour, it is probably easier and
> simpler to use an enum in this case.
>
> import java.awt.*;
>
> public class test {
> enum Colours {
> Red(Color.RED),Blue(Color.BLUE),Green(Color.GREEN);
>
> private final Color value;
>
> Colours(Color value) {
> this.value = value;
> }
>
> public Color value() {
> return value;
> }
> }
>
> public static void main(String... args) {
> System.out.println(Colours.Red);
> System.out.println(Colours.Red.value());
>
> System.out.println();
>
> for (Colours colour : Colours.values())
> System.out.println(colour+":"+colour.value());
> }
> }
>
> C:\Users\Knute Johnson>java test
> Red
> java.awt.Color[r=255,g=0,b=0]
>
> Red:java.awt.Color[r=255,g=0,b=0]
> Blue:java.awt.Color[r=0,g=0,b=255]
> Green:java.awt.Color[r=0,g=255,b=0]

I did not know that you could use enums in that way.

But I see two problems.
1. You can not change the list of colours. (Not a problem in my case.)
2. Officially enums should be all caps.

The code is now:
    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 enum Colours {
            Blue(Color.BLUE),
            Red(Color.RED),
            Orange(Color.ORANGE),
            Yellow(Color.YELLOW),
            Green(Color.GREEN),
            Cyan(Color.CYAN),
            Magenta(Color.MAGENTA),
            Brown(new Color(0x9C, 0x5D, 0x52)),
            ;

            private final Color value;

            Colours(Color value) {
                this.value = value;
            }

            public Color value() {
                return value;
            }
        }

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

        //private functions
        private Colours getCurrentColour() {
            return Colours.values()[currentColour];
        }

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

        private void updateColour(Colours newColour) {
            label.setForeground(newColour.value());
            label.setText(description + " : " + newColour);
        }

        // private variables
        private Color       background      = Color.LIGHT_GRAY;
        private int         currentColour   = 0;
        private int         delay           = 4000;
        private String      description     = "JTextArea Color Change Example";
        private JLabel      label;

    }

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

[toc] | [prev] | [next] | [standalone]

#30403

Op Monday 24 Nov 2014 18:19 CET schreef Knute Johnson:

> Eric gave you all the biggies, I'll nit-pick a little.
>
> From the docs, JFrame:
>
> "As a convenience, the add, remove, and setLayout methods of this
> class are overridden, so that they delegate calls to the
> corresponding methods of the ContentPane. For example, you can add a
> child component to a frame as follows:
>
> frame.add(child);"
>
> so you can skip the call to getContentPane().

As I understood it, this was the only way to change the background
colour of the frame. (I want it to be the same as from the label.)


> You first call pack() and the call setSize() on your frame. Only one
> is really necessary.

I want a line above and below the label. This was in my opinion the
 easiest way to accomplish that.


> tricky. Using GridBagLayout as you did, causes the component to be
> sized to its preferredSize and then put in the middle of the frame.
> Using BorderLayout, the default, will cause the content to fill the
> area of the frame that is available.

I think that in this case it was the good thing to do, but I'll look
into your suggestion.


> Very small nit: The frame title is a parameter of the constructor.
> You could have used:
>
> super(description)  as the first line.

When I try that I get:
    ColourCycling.java:10: error: cannot reference description before supertype constructor has been called
            super(description);


> In updateColour() you call label.setText(""), then change the color
> and then set the text again. What you were seeing with the text
> changing and then the color changing were very likely artifacts from
> updating the JLabel from other than the event dispatch thread.

In the latest version I was not doing that anymore. I am using the EDT
there.


> Another very small nit: you used the older versions of color
> constants with the lower case field names. In 1.4 new constants were
> added with upper case names to match current convention.

Every where I looked both where defined and most often the lowercase
used, but I'll change it.


> Here is a JLabel that displays the current time. Looks very similar
> to your example. I added an empty border to give you an example of
> how to do that too.

I'll look into it. Thanks.

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

[toc] | [prev] | [next] | [standalone]

#30404

On 11/24/2014 15:11, Cecil Westerhof wrote:
> Op Monday 24 Nov 2014 18:19 CET schreef Knute Johnson:
>>  From the docs, JFrame:
>>
>> "As a convenience, the add, remove, and setLayout methods of this
>> class are overridden, so that they delegate calls to the
>> corresponding methods of the ContentPane. For example, you can add a
>> child component to a frame as follows:
>>
>> frame.add(child);"
>>
>> so you can skip the call to getContentPane().

That was my mistake, I was looking at the wrong line.

Another way around that is to extend JComponent rather than JFrame and 
add the component to the frame.  For a while people were fairly adamant 
about not extending JFrame but that's worn off a little lately.  I often 
extend JFrame for simple programs.  It used to be that we thought we 
might run some of them in applets but not so much any more.

>
>> Very small nit: The frame title is a parameter of the constructor.
>> You could have used:
>>
>> super(description)  as the first line.
>
> When I try that I get:
>      ColourCycling.java:10: error: cannot reference description before supertype constructor has been called
>              super(description);

The error on the super call is because description is local rather than 
a class field.

Have you looked at findbugs?  It's a really cool tool to find poorly 
designed code, orphans and other things.  Its free.

http://findbugs.sourceforge.net/

-- 

Knute Johnson

[toc] | [prev] | [next] | [standalone]

#30406

In article <87zjbg97c5.fsf@Equus.decebal.nl>, Cecil Westerhof 
(Cecil@decebal.nl) says...

> 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.
> 
>     import java.awt.*;
>     import java.util.*;
> 
>     import javax.swing.*;
> 
>     @SuppressWarnings("serial")
>     public class ColourCycling extends JFrame {

First lesseon: Never derive from JFrame, always derive views from JPanel 
and put that into a frame.
Reason: You may want use this component elsewhere (inside another panel 
probably).

>         // public functions
>         public ColourCycling() {
>             java.util.Timer colourTimer = new java.util.Timer();
> 
>             getContentPane().setBackground(background);
>             setLayout(new GridBagLayout());
>             label.setFont(new Font("serif", Font.BOLD, 25));
>             add(label);
>             pack(); 

Lesson two: pack() is useless, if you set the rame size manually, after 
packing:

>             setSize(getWidth(), 3 * getHeight());

Lesson three: Never set the size of the frame directly. pack(9 should be 
enough.
The frame size is the size of the frame including its decorations. 
Different systems have thicker frames, smaller or larger fonts in the 
frame title. If you set the size directly, you might cut off the bottom 
or right part of the content,if the frame decorations are larger than 
expected, or you leave space, if it is smaller.
You want the frame to wrap around a panel, let the layout manager figure 
out the site of the content. If realy necessary, set the preferredSize 
of a component. Use pack() after you have built your component hierarchy 
and leave the frame size alone.

>             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) {
>             new ColourCycling();
>         }

Lesson four. Never create or manipulatre Components outside of the Event 
Dispatching Thread.
You want:
EvenQueue.invokeLater(new Runnable(){
 public void run(){
  JFrame frame = new JFrame();
  frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
  frame.getContentpane().add(new ColourCycling());//assume CC ext JPanel
  frame.pack();
  frame.setVisible(true);
 }
});



 
>         // private classes
>         private class Colour {


Lesson five: Naming a private class the same as an API class works, but 
is not well readable. Find a better name, for example 
"ColorWithDescription" or "NamedColor" or "ChoosableColor" or whatever 
is appropriate for its use.

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

> 
>         private class SwitchColour extends TimerTask {
>             public void run() {
>                 updateColour(getNextColour());
>             }
>         }

Back to lesson four: Never create or manipulate Components outside of 
the Event Dispatching Thread. "invokeLater" is your friend:

Since the TimerTask is executed by a java.util.Timer, it is executed on 
e thread other than the Swing Event Dispatching Thread, where everything 
has to be rendered. You want to push the operation to the event 
dispatching thread:

EventQueue.invokeLater(new Runnable(){
 public void run(){
  updateColour(getNextColour());
 }
});

By the way: It doesn't matter, if you use "EventQueue.invokeLater" or 
"SwingUtilities.invokeLater". 

Kind regards,
Wanja


-- 
..Alesi's problem was that the back of the car was jumping up and down 
dangerously - and I can assure you from having been teammate to 
Jean Alesi and knowing what kind of cars that he can pull up with, 
when Jean Alesi says that a car is dangerous - it is. [Jonathan Palmer]

[toc] | [prev] | [next] | [standalone]

#30408

On 11/25/2014 1:58 PM, Brixomatic wrote:
> In article <87zjbg97c5.fsf@Equus.decebal.nl>, Cecil Westerhof
> (Cecil@decebal.nl) says...
>> [...]
>>          // private classes
>>          private class Colour {
>
> Lesson five: Naming a private class the same as an API class works, but
> is not well readable. [...]

     Have u overlooked something?

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

#30414

In article <m52log$vq4$1@dont-email.me>, Eric Sosman (esosman@comcast-
dot-net.invalid) says...
> 
> On 11/25/2014 1:58 PM, Brixomatic wrote:
> > In article <87zjbg97c5.fsf@Equus.decebal.nl>, Cecil Westerhof
> > (Cecil@decebal.nl) says...
> >> [...]
> >>          // private classes
> >>          private class Colour {
> >
> > Lesson five: Naming a private class the same as an API class works, but
> > is not well readable. [...]
> 
>      Have u overlooked something?

Are you really trying to nitpick on American vs. English spelling?

Kind regards,
Wanja

-- 
..Alesi's problem was that the back of the car was jumping up and down 
dangerously - and I can assure you from having been teammate to 
Jean Alesi and knowing what kind of cars that he can pull up with, 
when Jean Alesi says that a car is dangerous - it is. [Jonathan Palmer]

[toc] | [prev] | [next] | [standalone]

#30417

On 11/25/2014 7:58 PM, Brixomatic wrote:
> In article <m52log$vq4$1@dont-email.me>, Eric Sosman (esosman@comcast-
> dot-net.invalid) says...
>>
>> On 11/25/2014 1:58 PM, Brixomatic wrote:
>>> In article <87zjbg97c5.fsf@Equus.decebal.nl>, Cecil Westerhof
>>> (Cecil@decebal.nl) says...
>>>> [...]
>>>>           // private classes
>>>>           private class Colour {
>>>
>>> Lesson five: Naming a private class the same as an API class works, but
>>> is not well readable. [...]
>>
>>       Have u overlooked something?
>
> Are you really trying to nitpick on American vs. English spelling?

     I'm merely saying that "Colour" is not "the same as an API class."
Nor do I think "Colour" and "Color" would be easily confused: Whether
one is Left- or Right-Pondian, one will read smoothly while the other
will jar, and neither is likely to be mistaken for the other.

     Let's harbour no groudges, oukay?

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

#30446

In article <m54lnv$e8r$1@dont-email.me>, Eric Sosman (esosman@comcast-
dot-net.invalid) says...

> > Are you really trying to nitpick on American vs. English spelling?
> 
>      I'm merely saying that "Colour" is not "the same as an API class."
> Nor do I think "Colour" and "Color" would be easily confused: Whether
> one is Left- or Right-Pondian, one will read smoothly while the other
> will jar, and neither is likely to be mistaken for the other.

Assuming your code is only read by native speakers is, erm, brave.

Kind regards,



-- 
..Alesi's problem was that the back of the car was jumping up and down 
dangerously - and I can assure you from having been teammate to 
Jean Alesi and knowing what kind of cars that he can pull up with, 
when Jean Alesi says that a car is dangerous - it is. [Jonathan Palmer]

[toc] | [prev] | [next] | [standalone]

#30450

On 11/29/2014 1:43 PM, Brixomatic wrote:
> In article <m54lnv$e8r$1@dont-email.me>, Eric Sosman (esosman@comcast-
> dot-net.invalid) says...
>
>>> Are you really trying to nitpick on American vs. English spelling?
>>
>>       I'm merely saying that "Colour" is not "the same as an API class."
>> Nor do I think "Colour" and "Color" would be easily confused: Whether
>> one is Left- or Right-Pondian, one will read smoothly while the other
>> will jar, and neither is likely to be mistaken for the other.
>
> Assuming your code is only read by native speakers is, erm, brave.

     Well, then we get to the question of "How different should names
be in order that non-native readers won't confuse them?"  And perhaps
we might consult the Java API itself for some guidance.  Therein, I
find two classes named AclEntry, two named Action, two named
AlreadyBoundException, two named Annotation, two named AnySeqHelper,
two named Array, two named ArrayType, two named AttributeSet, two named
AuthenticationException, *three* named Attributes, *three* named
AttributeList, *four* named Attributes ...

     ... and that's only the A's. It's clear that Oracle expects a
Java programmer to be sensitive to differences a good deal subtler
than Color vs. Colour -- for good or for ill.  But that's what name
spaces are for, isn't it?

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | comp.lang.java.programmer

csiph-web