Another JUnit scenario

[Rhino]

What is a good JUnit test for a method that takes no input parameters,
throws no exceptions, and returns no values but only writes information to
a console?

In addition to the getLocales() method which we're discussing in another
thread, I have a displayLocales() method which begins by calling getLocales
(), then writes the information it got from getLocales() to the console.

I'm at a loss in trying to think of something to test in a JUnit test for
this method.

Is there some way to prove that the console was written to? Is there some
way to intercept the output to be sure that the right things were written?
Or that at least the right number of lines was written and a few of the key
Locales appeared on the console? Or is none of that necessary for a method
with these characteristics?

 

[Arne Vajhøj]

What is a good JUnit test for a method that takes no input parameters,
throws no exceptions, and returns no values but only writes information to
a console?

In addition to the getLocales() method which we're discussing in another
thread, I have a displayLocales() method which begins by calling getLocales
(), then writes the information it got from getLocales() to the console.

I'm at a loss in trying to think of something to test in a JUnit test for
this method.

Is there some way to prove that the console was written to? Is there some
way to intercept the output to be sure that the right things were written?
Or that at least the right number of lines was written and a few of the key
Locales appeared on the console? Or is none of that necessary for a method
with these characteristics?

Because such a method is more demo style than library style,
then I am not sure that it is suited for unit testing at all.

But if you want to do it, then just catch output and compare
it to what you expect.

Arne

 

[Arne Vajhøj]

Because such a method is more demo style than library style,
then I am not sure that it is suited for unit testing at all.

But if you want to do it, then just catch output and compare
it to what you expect.

PrintStream save = System.out;
ByteArrayOutputStream baos = new ByteArrayOutputStream();
PrintStream pw = new PrintStream(baos);
System.setOut(pw);
// do something
System.setOut(save);
pw.flush();
// get output from baos.toString()

Arne

 

[Rhino]

PrintStream save = System.out;
ByteArrayOutputStream baos = new ByteArrayOutputStream();
PrintStream pw = new PrintStream(baos);
System.setOut(pw);
// do something
System.setOut(save);
pw.flush();
// get output from baos.toString()

Arne

Thanks for the suggestions (and the details on how I could capture the
output)!

 

[Rhino]

This sort of thing illustrates a somewhat subtle advantage of test
driven design. There are often small differences in an interface
design that make all the difference in how easy it is to test. Test
driven design naturally leads to a preference for designs that can be
tested over designs that are harder to test.

Maybe I'm reading too much into your remarks but are you hinting that
maybe I shouldn't even have such a method? Just stick with a getLocales()
method and leave the classes that use it to have their own code for
displaying the Locales? Because that's what I've been considering as I
struggled with how to test displayLocales().

It _is_ just a convenience method, a way to do in one line what would
have taken four or five lines if it didn't exist. As such, its existence
is no more defensible than any other convenience method. Then again, it
IS convenient - I've used it very occasionally to remind myself which
Locales are available in my JVM - so I might miss it (slightly) if it
weren't there.

For example, many output methods could be written to take a
PrintWriter or PrintStream parameter. Methods written that way are
easy to test by giving them a PrintWriter based on a StringWriter or a
PrintStream based on a ByteArrayOutputStream.

One such example is Properties, which has an overloaded list() method
that writes to either a PrintWriter or a PrintStream.... If I recall
correctly, it was this class or something similar which inspired me to
write displayLocales() in the first place several years back.

You still need a system level test procedure that checks that output
is going to the right destination, but that test can even be manual,
and does not need to deal with the unit test level details.

Writing the tests after defining the method you are stuck with
workarounds such as temporarily replacing System.out.

So, if I keep the method, I should capture the System.out and test it the
same way I did for getLocales: verify that the right number of Locales
was displayed and that a handful of the key ones, like en_US, were among
those displayed? I can do that, now that Arne showed me the technique
elsewhere in this thread.

 

[Daniel Pitts]

What is a good JUnit test for a method that takes no input parameters,
throws no exceptions, and returns no values but only writes information to
a console?

In addition to the getLocales() method which we're discussing in another
thread, I have a displayLocales() method which begins by calling getLocales
(), then writes the information it got from getLocales() to the console.

I'm at a loss in trying to think of something to test in a JUnit test for
this method.

Is there some way to prove that the console was written to? Is there some
way to intercept the output to be sure that the right things were written?
Or that at least the right number of lines was written and a few of the key
Locales appeared on the console? Or is none of that necessary for a method
with these characteristics?

Instead of writing to the console, your method should write to a
stream/writer/etc... which was configured in the constructor of the
object which the method was called on.

Then, you can create a new instance of that class, pass in a
StringWriter, and later retrieve the string that was written to it, and
verify the contents.

 

[Rhino]

Instead of writing to the console, your method should write to a
stream/writer/etc... which was configured in the constructor of the
object which the method was called on.

Then, you can create a new instance of that class, pass in a
StringWriter, and later retrieve the string that was written to it,
and verify the contents.

Or pass a reference to the stream/writer to the method, like
Properties.list() does?

Thanks for the suggestion. I would like to make my code as good as I can
and if writing to a stream or writer is better, I'm game to do that.

 

[Rhino]

Instead of writing to the console, your method should write to a
stream/writer/etc... which was configured in the constructor of the
object which the method was called on.

Then, you can create a new instance of that class, pass in a
StringWriter, and later retrieve the string that was written to it,
and verify the contents.

Just to be clear, let's say that the class containing my displayLocales()
method is called LocalizationUtils and it is a static factory class. Are
you saying that the stream or writer or whatever should be configured in
the constructor of LocalizationUtils or in the class which CALLS
LocalizationUtils? I'm assuming you mean the latter but I just want to be
sure.

 

[Lew]

Just to be clear, let's say that the class containing my displayLocales()
method is called LocalizationUtils and it is a static factory class. Are
you saying that the stream or writer or whatever should be configured in
the constructor of LocalizationUtils or in the class which CALLS
LocalizationUtils? I'm assuming you mean the latter but I just want to be
sure.

He was saying in the constructor of LocalizationUtils. However, static
factory classes have their own rules.

A (usually - always the disclaimer) better pattern is to include the Locale as
an argument to the factory method. Static state is an antipattern.

public class LocalizationUtils
{
/** Don't forget the private constructor! */
private LocalizationUtils(){}

/**
* Foo factory with default Locale.
* @return Foo instance with default Locale.
*/
public static Foo getFooInstance()
{
return new SubtypeOfFoo();
}

/**
* Foo factory with custom Locale.
* @param locale custom Locale.
* @return Foo instance with custom Locale.
*/
public static Foo getFooInstance( Locale locale )
{
return new SubtypeOfFoo( locale );
}
}

Remember, the only hard and fast rule of programming is that there are no hard
and fast rules of programming.

 

[Lew]

Remember, the only hard and fast rule of programming is that there are
no hard and fast rules of programming.

I posted this in the wrong thread - should've been in "Design Questions about
static factory classes".

Seems that a hard and fast rule about my posts is that I realize my mistake
just after pressing Send.

 

[Rhino]

He was saying in the constructor of LocalizationUtils. However,
static factory classes have their own rules.

A (usually - always the disclaimer) better pattern is to include the
Locale as an argument to the factory method. Static state is an
antipattern.

public class LocalizationUtils
{
/** Don't forget the private constructor! */
private LocalizationUtils(){}

/**
* Foo factory with default Locale.
* @return Foo instance with default Locale.
*/
public static Foo getFooInstance()
{
return new SubtypeOfFoo();
}

/**
* Foo factory with custom Locale.
* @param locale custom Locale.
* @return Foo instance with custom Locale.
*/
public static Foo getFooInstance( Locale locale )
{
return new SubtypeOfFoo( locale );
}
}

Remember, the only hard and fast rule of programming is that there are
no hard and fast rules of programming.

(I saw your note that you meant to put this in the other thread. I hope
you'll still look back here for my followup though....)

I'm REALLY getting confused now. Do you mean for Foo to be the
constructor for the stream or writer that Daniel suggested? And why would
a non-default Locale call for a subtype of Foo, rather than Foo itself??

I was looking at the source code for the Properties class, specifically
the list() methods (I finally took a minute to find out where the source
code in the Java API is again) and I didn't see any contructors for
writers or streams in there. Is Properties not a good class to imitate
for my situation then?

 

[Lew]

I'm REALLY getting confused now. Do you mean for Foo to be the
constructor for the stream or writer that Daniel suggested? And why would

No. I mean for some subtype of 'Foo', possibly 'Foo' itself, to be the actual
type of the constructed instance, regardless of whether you specify the
'Stream' or 'Locale' or any other attribute.

a non-default Locale call for a subtype of Foo, rather than Foo itself??

'Locale' or 'Stream' or any other attribute for which the factory is
responsible, the principles are the same.

What makes you think that the non-default 'Locale' setting has anything to do
with using a subtype? I showed use of a subtype in both cases.

If you are only constructing 'Foo' instances and never different implementors
of 'Foo', you probably don't need a factory class and should just use 'Foo'
constructors.

One of the main purposes of a factory class is to provide a hidden
implementation of the constructed type. The returned type in such cases is
nearly always an interface.

You never answered my question about why you wanted to use a static factory
method, let alone a factory class, in the first place. The question was
neither arbitrary nor insignificant.

 

[Arne Vajhøj]

He was saying in the constructor of LocalizationUtils. However, static
factory classes have their own rules.

A (usually - always the disclaimer) better pattern is to include the
Locale as an argument to the factory method. Static state is an
antipattern.

public class LocalizationUtils
{
/** Don't forget the private constructor! */
private LocalizationUtils(){}

/**
* Foo factory with default Locale.
* @return Foo instance with default Locale.
*/
public static Foo getFooInstance()
{
return new SubtypeOfFoo();
}

/**
* Foo factory with custom Locale.
* @param locale custom Locale.
* @return Foo instance with custom Locale.
*/
public static Foo getFooInstance( Locale locale )
{
return new SubtypeOfFoo( locale );
}
}

Remember, the only hard and fast rule of programming is that there are
no hard and fast rules of programming.

I would prefer a non static field + getter + setter in the factory
over the arguments to the get instance methods.

IF you need to get many objects different places in the code
then configuring the factory once may require less code and
may work better if the factory is retrieved via something
like Spring.

Arne

 

[Arne Vajhøj]

Rhino wrote:
...
...

In general, you should not imitate classes, such as Properties, that
were designed very early in the life of Java. Many of them suffer from
at least one of two problems:

1. The initial Java API writers may have been very smart, but the
collective learning of the Java community over the years has often led
to better techniques.

In particular, they tend to depend too much on inheritance, rather than
composition. Making Properties extend Hashtable creates problems.

2. They are limited in their use of features added to the language since
they were defined.

Even if one were to expose the map aspects of Properties, it would
have been defined to implement Map<String,String>, rather than extend
Hashtable, if the Map interface and generics had existed when it was
defined. The Map optional methods that are undesirable for Properties,
put and putAll, would have thrown UnsupportedOperationException.

The Properties class has many unusual features - the parsing
of strings are also unusual.

But it is a very practical class. It is easy to use. Any beginner
can use it. Those that want something more sophistcated can use
one of the XML API's or even the dreaded preferences API (or
develop something themselves for pre-1.4).

This type of easy is what makes many developers like
the MS stuff. MS have a ton of practical but not
super-OO stuff.

Arne

 

[Rhino]

No.  I mean for some subtype of 'Foo', possibly 'Foo' itself, to be the actual
type of the constructed instance, regardless of whether you specify the
'Stream' or 'Locale' or any other attribute.

Sorry, that's just a jumble of words to me. I don't mean that you are
being unclear but that I'm afraid I don't do very well when everything
is put in very general terms like 'Foo', especially if I'm not clear
on what kind of thing Foo is supposed to be. And the fact that we've
got all these threads going with umpteen different considerations
expressed, some of which I only half understand, is definitely
complicating things.... And now my mail reader is refusing to connect!
Arrrggghhhh!! Okay, switching to Google Groups.....

'Locale' or 'Stream' or any other attribute for which the factory is
responsible, the principles are the same.

What makes you think that the non-default 'Locale' setting has anything to do
with using a subtype?  I showed use of a subtype in both cases.

If you are only constructing 'Foo' instances and never different implementors
of 'Foo', you probably don't need a factory class and should just use 'Foo'
constructors.

One of the main purposes of a factory class is to provide a hidden
implementation of the constructed type.  The returned type in such cases is
nearly always an interface.

You never answered my question about why you wanted to use a static factory
method, let alone a factory class, in the first place.  The question was
neither arbitrary nor insignificant.

Actually, I did answer it a few hours ago on one of the other threads.
I'm darned if I know which one at this point; there are so many
threads going now with so many subthreads that I am absolutely dizzy
keeping up wiht them and figuring out which ones I have
"answered" (affirmed that I understood or asked a followup or
clarifying question) and which ones I've missed.

Rather than tracking it down right now or risking paraphrasing my
remarks inaccurately and causing further confusion, it would be
helpful to _me_ to simply post LocalizationUils and/or
LocalizationUtils2 right now. They are quite brief and they would give
me some concrete reference points in asking questions. Basically, I
want to ask if I have done things correctly now and identify any
things that still need work. LocalizationUtils is satisfactory, I
should then be able to make equivalent changes on the other utility
classes.

I've replaced displayLocales(), which wrote to the console, with
writeLocales(PrintWriter) and writeLocales(PrintStream).

By the way, I have the JUnit tests doing all (or most?) of what
various people suggested. Those tests all give green checkmarks so I
_think_ I've proven that my functionality all works. With regards to
the writeXXX methods that write the Locales list, my test cases write
them to a file, then read them back from the file and parse them
sufficiently to verify that all 152 Locales are present and that
en_CA, en_US, fr_FR and de_DE are all there.

I'm just not sure if the overall design in right and I don't think I'm
understanding the right place to create the Stream or Writer that I
need.

Okay, let's just dive in. Here is LocalizationUtils2, with all the
imports, comments and javadocs stripped out for brevity:

===================================================================
public class LocalizationUtils2 {

static final String CLASS_NAME = "LocalizationUtils";
private final static String MSG_PREFIX =
"ca.maximal.common.utilities.Resources.LocalizationUtilsMsg";
public static final String HORIZONTAL_SEPARATOR = "\t";
StringUtils stringUtils = null;
private ResourceBundle locMsg = null;
private MessageFormat msgFmt = new MessageFormat("");

private Locale locale = null;
private LocalizationUtils2() {

locale = Locale.getDefault();
locMsg = getResources(locale, MSG_PREFIX);
msgFmt.setLocale(locale);
}

public static LocalizationUtils2 getInstance() {

return new LocalizationUtils2();
}

public String greeting() {

Object[] msgArgs = {};
msgFmt.applyPattern(locMsg.getString("msg012"));
return (msgFmt.format(msgArgs));
}

public SortedMap<String, Locale> getLocales() {

SortedMap<String, Locale> sortedLocales = new TreeMap<String,
Locale>();

for (Locale oneLocale : Locale.getAvailableLocales()) {
sortedLocales.put(oneLocale.getDisplayName(locale), oneLocale);
}

return sortedLocales;
}

public void writeLocales(PrintStream out) {

SortedMap<String, Locale> locales = getLocales();
int longestLocaleName = 0;
for (String oneLocale : locales.keySet()) {
if (oneLocale.length() > longestLocaleName) {
longestLocaleName = oneLocale.length();
}
}

stringUtils = StringUtils.getInstance();
for (String oneLocale : locales.keySet()) {
out.println(stringUtils.pad(oneLocale, ' ', 'T',
longestLocaleName) + HORIZONTAL_SEPARATOR +
locales.get(oneLocale));
}
out.flush();
}

public void writeLocales(PrintWriter out) {

SortedMap<String, Locale> locales = getLocales();
int longestLocaleName = 0;
for (String oneLocale : locales.keySet()) {
if (oneLocale.length() > longestLocaleName) {
longestLocaleName = oneLocale.length();
}
}
stringUtils = StringUtils.getInstance();
for (String oneLocale : locales.keySet()) {
out.println(stringUtils.pad(oneLocale, ' ', 'T', longestLocaleName) +
HORIZONTAL_SEPARATOR + locales.get(oneLocale));
}
out.flush();
}

public ResourceBundle getResources(Locale currentLocale, String
listBase) {

final String METHOD_NAME = "getResources()";
ResourceBundle locList = null;
try {
locList = ResourceBundle.getBundle(listBase, currentLocale);
} catch (MissingResourceException mr_excp) {
Object[] msgArgs = {CLASS_NAME, METHOD_NAME, listBase,
currentLocale.toString(), mr_excp};
msgFmt.applyPattern(locMsg.getString("msg011"));
throw new IllegalArgumentException(msgFmt.format(msgArgs));
}

return (locList);

}
}

===================================================================

I initially had two private constructors, the one you see here which
takes no Locale parameter, and a second one which did. I initially had
two getInstance() methods, one for each constructor, so one had no
parameter and one took Locale as its parameter. I also had a
getLocale() and setLocale(). This was the "everything INCLUDING the
kitchen sink" version and allowed people to invoke the class with the
default Locale or a specified one and then to change Locales anytime
they wanted. Some of the discussion here persuaded me that this was
too much so I stripped out the constructor and getInstance() that used
the Locale parameter and removed getLocale() and setLocale(). (And
yes, I have an open mind on this and could be persuaded to restore
them; I've kept the previous version of the class around just in case
I want to do this.)

I know now that the user will get messages according to his
user.language and user.country settings. (I created the frivolous
greeting() method and made ResourceBundles just to demonstrate this. I
made ResourceBundles for English, French and German containing a
single greeting (Merry Christmas) in each of those languages with the
key of msg012. I changed my user.language and user.country settings in
the JVM via the run profile in Eclipse and verified that, depending on
which values those settings have, the user will get the greeting in
that language.) Therefore, I am supporting users that speak various
languages and can easily add more. Users can have whichever of the
supported languages they want simply by setting user.language and
user.country. Users who want to switch on the fly from English to
German and then to French can't do that; they are out of luck. But the
ability to switch on the fly is probably overbuilding anyway. That's
my (tentative) executive decision on that

Getting expert reactions to this aspect of the class is my main issue
at this point.


The lesser issue is the whole business about where the Writer or
Stream used by writeLocales() should be done. At the moment, I create
the Writer or Stream in my JUnit test case. It seems to me that anyone
writing a class calling my utility class would expect to create the
Writer or Stream there, not that I would do it for them. But I'm
definitely willing to keep an open mind on that!

I suppose my biggest reservation about creating the Writer or Stream
in my class is the fear that it limits what the caller gets. For
instance, if I create a Stream that writes to a physical file but the
caller wants to redirect the output to the console, aren't I limiting
his options unnecessarily?

If I should create the Writer or Stream in LocalizationUtils2, what
characteristics should I give it?

Since it seems we've decided against using Properties as a model, can
someone suggest a better model that does things the way the suggester
(Daniel?) meant?

Lastly, I would be happy to post the test cases for the class here to
get people's comments if they'd like to see them. I don't expect that
they'd be controversial but there might well be things that I could
have done more efficiently. For instance, I test the writeLocales()
methods by sending the output to a flat file, then read them back and
parse them to be sure that the right number of Locales was written and
that four specific Locales were present. That code is a bit bulkier
than I've had to do in other tests so maybe there is a better way.....

Would it help or confuse things if I replicated this post on the OTHER
threads that are discussing these issues, particularly the Design
Questions for Static Factory Methods one?

 

[Lew]

Actually, I did answer it a few hours ago on one of the other threads.

You answered it over two and half hours after I posted the above, in "Design
Questions about static factory classes". I didn't have my USB crystal ball
attached when I wrote that you hadn't. So sorry.

You answered, essentially, that you didn't know why you were using a static
factory and were just applying the principle by rote, cargo-cult style.

That's not programming.

Remember, the only hard and fast rule in programming is that there are no hard
and fast rules in programming. You cannot just go apply a hammer to
everything hoping that you're hitting a nail. Sometimes you're hitting a
crystal vase, with bad results.

You must THINK.

I'm darned if I know which one at this point; there are so many
threads going now with so many subthreads that I am absolutely dizzy

Rhino, it only took me five minutes of digging around to find the particular
post. I guess I'm just not very lazy.

keeping up wiht them and figuring out which ones I have
"answered" (affirmed that I understood or asked a followup or
clarifying question) and which ones I've missed.

You should fix that.

===================================================================
public class LocalizationUtils2 {

static final String CLASS_NAME = "LocalizationUtils";

Interesting that you label something 'CLASS_NAME' that does not match the name
of the class in which it appears. Careless.

private final static String MSG_PREFIX =
"ca.maximal.common.utilities.Resources.LocalizationUtilsMsg";
public static final String HORIZONTAL_SEPARATOR = "\t";

Why is this 'public'?

StringUtils stringUtils = null;

Why do you redundantly set the member to 'null'? Why is it package-private
("default" access)?

private ResourceBundle locMsg = null;
private MessageFormat msgFmt = new MessageFormat("");

private Locale locale = null;
private LocalizationUtils2() {

locale = Locale.getDefault();

It's so weird. You explicitly set 'locale' to 'null', its default value
anyway, when you have no use for and no intention of ever having a use for
that value.

locMsg = getResources(locale, MSG_PREFIX);
Ditto.

msgFmt.setLocale(locale);
}

public static LocalizationUtils2 getInstance() {

return new LocalizationUtils2();

Usually when a class is a "utility" class, it has only static members and is
never instantiated.

}

public String greeting() {

Object[] msgArgs = {};
msgFmt.applyPattern(locMsg.getString("msg012"));
return (msgFmt.format(msgArgs));
}

public SortedMap<String, Locale> getLocales() {

SortedMap<String, Locale> sortedLocales = new TreeMap<String,
Locale>();

for (Locale oneLocale : Locale.getAvailableLocales()) {
sortedLocales.put(oneLocale.getDisplayName(locale), oneLocale);
}

return sortedLocales;
}

public void writeLocales(PrintStream out) {

SortedMap<String, Locale> locales = getLocales();
int longestLocaleName = 0;
for (String oneLocale : locales.keySet()) {
if (oneLocale.length()> longestLocaleName) {
longestLocaleName = oneLocale.length();
}
}

stringUtils = StringUtils.getInstance();

This looks wrong. You have defined 'stringUtils' to be part of the object's
state, i.e., a member variable, but you only use it like a local variable.
You should declare it as a local variable, not a member variable.

for (String oneLocale : locales.keySet()) {
out.println(stringUtils.pad(oneLocale, ' ', 'T',
longestLocaleName) + HORIZONTAL_SEPARATOR +
locales.get(oneLocale));

On the other hand, you use 'StringUtils' like a utility class. The methods
should be 'static' and you should not instantiate it.

That means no 'stringUtils' variable at all.

}
out.flush();
}

public void writeLocales(PrintWriter out) {

SortedMap<String, Locale> locales = getLocales();

Is the list of 'Locale's likely to change often, because you build it every
time you refer to it. That implies you expect the value to change between calls.

int longestLocaleName = 0;
for (String oneLocale : locales.keySet()) {
if (oneLocale.length()> longestLocaleName) {
longestLocaleName = oneLocale.length();
}
}
stringUtils = StringUtils.getInstance();
for (String oneLocale : locales.keySet()) {
out.println(stringUtils.pad(oneLocale, ' ', 'T', longestLocaleName) +
HORIZONTAL_SEPARATOR + locales.get(oneLocale));
}
out.flush();
}

public ResourceBundle getResources(Locale currentLocale, String
listBase) {

final String METHOD_NAME = "getResources()";
ResourceBundle locList = null;

Redundant assignment of 'null'.

try {
locList = ResourceBundle.getBundle(listBase, currentLocale);
} catch (MissingResourceException mr_excp) {
Object[] msgArgs = {CLASS_NAME, METHOD_NAME, listBase,
currentLocale.toString(), mr_excp};
msgFmt.applyPattern(locMsg.getString("msg011"));
throw new IllegalArgumentException(msgFmt.format(msgArgs));
}

return (locList);

}
}

===================================================================

...
Getting expert reactions to this aspect of the class is my main issue
at this point.

I started to address those matters (that I elided here) for you, but realized
at this point that you have been overwhelmed. You're posting to quickly and
too frequently; you clearly have not had time to practice with and assimilate
the information you've already obtained. I, for one, am loathe to barrage you
with more wisdom until you've achieved a plateau with what people have already
told you.

Go and practice.

Consider NOT calling the class by a name that includes "Util" or "Utils" if
it's instantiable - conventionally such a name implies a class with no state
and no instance members.

Now go, practice, think, assimilate and master what you've already been given.

Would it help or confuse things if I replicated this post on the OTHER
threads that are discussing these issues, particularly the Design
Questions for Static Factory Methods one?

Gods! Don't do that!

 

[Lew]

I would prefer a non static field + getter + setter in the factory
over the arguments to the get instance methods.

As long as you don't need the thing to be thread safe that's fine, too. Even
better, usually, is a non-static final field set in the factory's constructor
with only a getter. You instantiate a new instance of the factory each time
you need a different 'Locale' (or whatever the attribute(s) is/are).

IF you need to get many objects different places in the code
then configuring the factory once may require less code and
may work better if the factory is retrieved via something
like Spring.

I have a hard time grokking Spring.

 

[Arne Vajhøj]

As long as you don't need the thing to be thread safe that's fine, too.
Even better, usually, is a non-static final field set in the factory's
constructor with only a getter. You instantiate a new instance of the
factory each time you need a different 'Locale' (or whatever the
attribute(s) is/are).

I would leave the thread safeness to threads having different factory.

Constructor or setter does not differ much.

Just because in a few contexts no arg constructor and setter
is better I have a tendency to use that everywhere.

I have a hard time grokking Spring.

It is both overhyped and highly misused.

But I prefer it over homegrown pick class by
config frameworks.

Arne

 

[Rhino]

You answered it over two and half hours after I posted the above, in
"Design Questions about static factory classes". I didn't have my USB
crystal ball attached when I wrote that you hadn't. So sorry.

I meant no criticism of you at all with that remark Lew. Seriously. I
assumed that we had "crossed" and were just catching up with each other.

You answered, essentially, that you didn't know why you were using a
static factory and were just applying the principle by rote,
cargo-cult style.

That's not programming.

Hey, I'm not happy to have to give you that answer but I didn't want to
BS either. I've been taking in a LOT of information in the last few days
and some of it is turning things I thought I knew on their heads. I have
a lot to digest and my memory has never been great so sometimes I coast a
bit and do what I'm told and simply resolve to clear up the whys and
wherefores somewhere down the road. And I don't always get to that
point....

Remember, the only hard and fast rule in programming is that there are
no hard and fast rules in programming. You cannot just go apply a
hammer to everything hoping that you're hitting a nail. Sometimes
you're hitting a crystal vase, with bad results.

You must THINK.

I know that. I'm just not sure HOW to reason some of this stuff out.
Maybe I'm missing some fundamentals or something. Even the simplest
things sometimes turn out to be very complicated and non-obvious. In
fact, I'm fast coming to the point where I'm beginning to think NOTHING
in Java is simple, obvious, or intuitive....

But enough of my self-pity....

Rhino, it only took me five minutes of digging around to find the
particular post. I guess I'm just not very lazy.

And I probably am a little lazy at time. But I'm really burned out. We
have a three day weekend here and I had planned to code, code, code my
little heart out this weekend and finish my code portfolio. Instead, I've
mostly been in and out of this newsgroup trying to understand and absorb
all the things people are throwing at me. I've actually accomplished very
little of the coding I wanted to do. And now I find that most of what I
have is bad, maybe very very bad. So that big pile of code has turned
into a mountain.

You should fix that.

I'd love to. But this newsreader is the best one I could lay my hands on
after Outlook Express refused to post any more - for no reason I could
ever find - and it pretty much sucks a lot of the time. I can't spare any
time right now to find a better newsreader so that it is easier to keep
track of what I have already read and responded to. What I really SHOULD
do - in the usual 20/20 hindsight - is not post so damned many questions
all at the same time. It's a lot easier to follow one or two threads than
several....

Interesting that you label something 'CLASS_NAME' that does not match
the name of the class in which it appears. Careless.

Actually, once I get this code cleaned up - if I ever do - I plan to
ditch LocalizationUtils and rename LocalizationUtils2 to just plain
LocalizationUtils. So I'm just leaving the value wrong for now, knowing
that it will be right eventually.

Why is this 'public'?

I thought I'd let the JUnit test case access it directly. I use that
separator between the two columns in the writeLocales() methods so the
test case needs it to see how to split the lines to verify that the
correct classes are there.

Why do you redundantly set the member to 'null'? Why is it
package-private ("default" access)?

I long ago got into a habit of initializing everything to SOMETHING. I
got tired of compiler messages complaining that things were not
initialized. I initialize pretty much everything that can be initialized
to null if I don't have a specific value that it needs to be. I'm
guessing this is wrong too?

As for the access, I frequently don't know what level of access I should
set. If I know - or think I know - I'll code public or private or
whatever. But I don't know how to reason this out in every case. In those
cases, I might not set anything at all and take the default. Or maybe I
just didn't notice that I hadn't set a specific access level. That's
entirely possible too.

It's so weird. You explicitly set 'locale' to 'null', its default
value anyway, when you have no use for and no intention of ever having
a use for that value.

Sorry. I think I meant to delete Locale altogether after I made my
executive decisions about not having the extra constructor/getInstance
and setLocale()/getLocale(). Then again, one of my methods uses locale
and it still works. I must investigate why it still works when I don't
set it....

Ditto.

locMsg is being used in greeting() and getResources(). greeting(), of
course, is to be thrown away; it was just there for an experiment and
should be ignored....

Usually when a class is a "utility" class, it has only static members
and is never instantiated.

I think that's how my utility classes started out. I wasn't sure if that
was right and asked on this newsgroup. People asked me various questions,
I answered them as accurately as I could and the gist of the advice was
that I should go with this approach. I reworked the previous code
according to that advice. Now you're telling me that I was right before
and the fundamental design in WRONG???

}

public String greeting() {

Object[] msgArgs = {};
msgFmt.applyPattern(locMsg.getString("msg012"));
return (msgFmt.format(msgArgs));
}

public SortedMap<String, Locale> getLocales() {

SortedMap<String, Locale> sortedLocales = new TreeMap<String,
Locale>();

for (Locale oneLocale : Locale.getAvailableLocales()) {
sortedLocales.put(oneLocale.getDisplayName(locale), oneLocale);
}

return sortedLocales;
}

public void writeLocales(PrintStream out) {

SortedMap<String, Locale> locales = getLocales();
int longestLocaleName = 0;
for (String oneLocale : locales.keySet()) {
if (oneLocale.length()> longestLocaleName) {
longestLocaleName = oneLocale.length();
}
}

stringUtils = StringUtils.getInstance();

This looks wrong. You have defined 'stringUtils' to be part of the
object's state, i.e., a member variable, but you only use it like a
local variable. You should declare it as a local variable, not a
member variable.

In this class, StringUtils gets used in both of the writeLocales()
methods. I've always gone with the rule of thumb that if something is
used in one method, I define it locally but if it occurs within multiple
methods, I define it centrally within the class. So that's wrong too?

On the other hand, you use 'StringUtils' like a utility class. The
methods should be 'static' and you should not instantiate it.

That means no 'stringUtils' variable at all.

That's just dandy. If you had any idea how much time I've spent changing
all this code to work the way I was told to do it, only to find out it
was just fine before I asked.....

Is the list of 'Locale's likely to change often, because you build it
every time you refer to it. That implies you expect the value to
change between calls.

I expect it to change occasionally as newer JVMs are released. But I
don't know how to code a method to say "only look up the locales when the
JVM has been upgraded".

int longestLocaleName = 0;
for (String oneLocale : locales.keySet()) {
if (oneLocale.length()> longestLocaleName) {
longestLocaleName = oneLocale.length();
}
}
stringUtils = StringUtils.getInstance();
for (String oneLocale : locales.keySet()) {
out.println(stringUtils.pad(oneLocale, ' ', 'T',
longestLocaleName) +
HORIZONTAL_SEPARATOR + locales.get(oneLocale));
}
out.flush();
}

public ResourceBundle getResources(Locale currentLocale, String
listBase) {

final String METHOD_NAME = "getResources()";
ResourceBundle locList = null;

Redundant assignment of 'null'.

try {
locList = ResourceBundle.getBundle(listBase, currentLocale);
} catch (MissingResourceException mr_excp) {
Object[] msgArgs = {CLASS_NAME, METHOD_NAME, listBase,
currentLocale.toString(), mr_excp};
msgFmt.applyPattern(locMsg.getString("msg011"));
throw new IllegalArgumentException(msgFmt.format(msgArgs));
}

return (locList);

}
}

===================================================================

...
Getting expert reactions to this aspect of the class is my main issue
at this point.

I started to address those matters (that I elided here) for you, but
realized at this point that you have been overwhelmed. You're posting
to quickly and too frequently; you clearly have not had time to
practice with and assimilate the information you've already obtained.
I, for one, am loathe to barrage you with more wisdom until you've
achieved a plateau with what people have already told you.

Go and practice.

I can't disagree with you there. I AM overwhelmed.

Consider NOT calling the class by a name that includes "Util" or
"Utils" if it's instantiable - conventionally such a name implies a
class with no state and no instance members.

Now go, practice, think, assimilate and master what you've already
been given.

It's either that or go look for a job at McDonalds. I'm beginning to
think I know nothing at all about Java despite having coded in it since
1997....

Gods! Don't do that!

Okey dokey. That's why I asked.

My god, this is SO depressing. I thought I was ALMOST to the point where
I had some good code. But almost every blessed line is wrong, wrong,
wrong....

I wonder if McDonald's is hiring....

I definitely have to give up on the idea of having a portfolio of good
code in the next few days. I think I'm still months, maybe YEARS from
having anything decent to show. Like I said in the other thread, I need
to find an employer that sees some potential and beg them for a job so
that I can learn from people who know what they're doing. Maybe if I can
convince them that I love to write code, especially in Java, and that I
can at least get code to work, even if I have a thousand mistakes, and
that I really want to learn to do it right, they'll decide to give me a
chance. Then I can find a mentor and start to write good code, not this
apparent crap....

Now if I could just think of a reason why they'd hire me if they had the
chance of hiring someone who knew what they were doing already. It would
be one thing if I was fresh out of school and 20-something but those days
are way behind me....


I am up way past my bedtime and I think it's making me start to babble.
Time to call it a night. Maybe things will make more sense in the
morning.

Thanks for your honest appraisal. It obviously wasn't the good news I was
hoping for but it would have been a very bad thing indeed if you had told
me my code was great when it isn't. I really don't want to be the butt of
uproarious laughter with a potential employer. I can just picture my code
being printed and posted on a bulletin board with a caption that says
"Check this out guys; this idiot thinks he can write Java!"

 

[Arne Vajhøj]

Hey, I'm not happy to have to give you that answer but I didn't want to
BS either. I've been taking in a LOT of information in the last few days
and some of it is turning things I thought I knew on their heads. I have
a lot to digest and my memory has never been great so sometimes I coast a
bit and do what I'm told and simply resolve to clear up the whys and
wherefores somewhere down the road. And I don't always get to that
point....

I know that. I'm just not sure HOW to reason some of this stuff out.
Maybe I'm missing some fundamentals or something. Even the simplest
things sometimes turn out to be very complicated and non-obvious. In
fact, I'm fast coming to the point where I'm beginning to think NOTHING
in Java is simple, obvious, or intuitive....

Note that usenet is more an academic place than a job place
meaning that there are always some people willing to
find problems in a solution and come up with clever ways to
refine it.

In job context bosses want code delivered on tight schedules.

And I probably am a little lazy at time. But I'm really burned out. We
have a three day weekend here and I had planned to code, code, code my
little heart out this weekend and finish my code portfolio. Instead, I've
mostly been in and out of this newsgroup trying to understand and absorb
all the things people are throwing at me. I've actually accomplished very
little of the coding I wanted to do. And now I find that most of what I
have is bad, maybe very very bad. So that big pile of code has turned
into a mountain.

Maybe it is time to focus on writing some 90% good code and
let the last 10% come over time.

I definitely have to give up on the idea of having a portfolio of good
code in the next few days. I think I'm still months, maybe YEARS from
having anything decent to show. Like I said in the other thread, I need
to find an employer that sees some potential and beg them for a job so
that I can learn from people who know what they're doing. Maybe if I can
convince them that I love to write code, especially in Java, and that I
can at least get code to work, even if I have a thousand mistakes, and
that I really want to learn to do it right, they'll decide to give me a
chance. Then I can find a mentor and start to write good code, not this
apparent crap....

Now if I could just think of a reason why they'd hire me if they had the
chance of hiring someone who knew what they were doing already. It would
be one thing if I was fresh out of school and 20-something but those days
are way behind me....

Don't be so pessimistic.

Very few if any programmers are perfect.

The fact that you do not write perfect code is not a reason
not to hire you.

They would never hire anyone if they were so picky.

I can not see why they should not hire you instead of
a few hundred thousand other.

If you really want to improve your marketability then I think
you should work on "Getting some code working" and
Google/Wikipedia skills. Not the finer subtleties of what
is the best OOP style.

Arne