Path: csiph.com!v102.xanadu-bbs.net!xanadu-bbs.net!feeder.erje.net!eu.feeder.erje.net!news.roellig-ltd.de!open-news-network.org!fbe001.ams.xsnews.nl!feed.xsnews.nl!border04.ams.xsnews.nl!feeder02.ams.xsnews.nl!frontend-F09-05.ams.news.kpn.nl
From: Cecil Westerhof <Cecil@decebal.nl>
Newsgroups: comp.lang.java.programmer
Subject: Re: Could this be done better
Organization: Decebal Computing
References: <87zjbg97c5.fsf@Equus.decebal.nl> <m4vf1f$j03$1@dont-email.me> <m4vpb5$9ue$1@dont-email.me>
X-Face: "(y8cC@tg_12{">GF'UXTW]FHI2wMiZNrnf'1EFQ&O#$m:f#O7+7}kR<J%a^F2lh4[N~Yz4 nSp#c+aQo1b5=?HcNEkQ7QzF<])O3X4MDL/AYjys&*mt>,v+Pti8=Vi/Z"g^?b"E
X-Homepage: http://www.decebal.nl/
Date: Tue, 25 Nov 2014 00:11:42 +0100
Message-ID: <87bnnw6vo1.fsf@Equus.decebal.nl>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux)
Cancel-Lock: sha1:ggCmlS6zkVQwI6so9FPTQBOoGxM=
MIME-Version: 1.0
Content-Type: text/plain
Lines: 72
NNTP-Posting-Host: 77.171.42.144
X-Trace: 1416870836 news.kpn.nl 21160 77.171.42.144@kpn/77.171.42.144:54109
Xref: csiph.com comp.lang.java.programmer: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