Design Question

M

markspace

Rhino said:
And that's how the code was when I posted the original question....

Right, and no worries. Now that we know what you are really doing, we
can say "the original design looks ok."

To summarize the opinions presented here, I'd say:

1. No object state, "utility" methods: consider static methods and a
private constructor.

2. Requires object state: consider public constructor & instance variables.


OK, now I think I see what you are asking. "Should this class be made
"final"". My opinion: no. Never never make a class final unless you
have thought about it and are sure it must be final.

There's some extra design required when you DON'T make a class final,
but you should always go through that process. The reason is I think
called The Open-Closed Principle, and basically it means that you should
always leave open the possibility that someone will want to extend your
class. Try to let them if at all possible. Other principles aside
("Prefer composition to extension.") it's pernicious to actually prevent
extension unless you have a good reason. Imo.
 
M

markspace

Rhino said:
Again, can you clarify this a bit? You don't necessarily need to write
an example; just pointing to a document or tutorial that contains an
example would be just fine. I'm not sure where to look for such a
thing.


A lot of what is being discussed here is based on patterns from a book
called "Effective Java" by Joshua Bloch. It's basically "design
patterns for Java" and it's very good. You should pick it up. Whether
you use its ideas or not is up to you, but I think it would extend your
understanding of Java quite a bit.


The basics of the Factory pattern are:

1. A static method that is the Factory.

2. A private-ish class that is produced by the Factory method.


public class StringUtils {

private StringUtils() {}

public StringUtils getInstance() {
return new StringUtils();
}
}


That's the idea, the full pattern can be more complicated. The
reasoning behind it is that there are some things you can't do in a
constructor, so the method gives you a change to add additional code
after (or before) the constructor is invoked. It's basically an
additional layer of encapsulation on top of the constructor. You should
look at the Java API and notice that a lot of the new-ish classes use
this kind of Factory pattern. Calendar, for example, and some of the
new Collections, like EnumSet.
 
S

Stefan Ram

markspace said:
1. No object state, "utility" methods: consider static methods and a
private constructor.

Even without state, one might want to implement interfaces.
Java does not provide interfaces with static methods.
 
M

markspace

Stefan said:
Even without state, one might want to implement interfaces.
Java does not provide interfaces with static methods.


True. The key here (for me anyway) is "utility methods." Short, well
understood algorithms that are unlikely to change, other than globally.
Example:

Math.cos( double ) ...

It's pretty unlikely you'd ever change the definition of a cosine. The
definition of a cosine is mathematically pretty well understood. If you
did change its implementation, you'd likely want those changes to affect
all code globally.

Many designs by users (rather than writers of the Java API) need the
flexibility of an interface because their code is in fact going to
change more often, every time the spec changes. So I agree that it's
important to point out interfaces in this context.

OTOH, the OP's example of "I have some string methods I think will make
my life easier" is a pretty good example of some low-level methods that
really might be best if implemented as static utility methods. They
probably don't appear on the spec, they're just there to make the
implementer's task easier.
 
A

Arne Vajhøj

Pointless code is harmful by virtue of its pointlessness.

You attached a cost of zero to pointless code, but its cost is larger
than zero. "Pointless" means that it provides no benefit, ergo cost
exceeds benefit.

It seems clear that instantiation or (especially) inheritance of a
utility class is pointless, of no benefit. It has a cost in extra code
coupling, reduction in self-documentation, muddier expression of type
relationships and just plain extra stuff to scratch a maintainer's
"wtf?" nerve over the life of the software. The burden is to justify its
existence, not the mandate against it.

The existence of code to prevent instantiation and inheritance has a
cost. In context of a conservative coding style that armors against
error, the burden of the cost is shared by the commitment to code
safety, but it still is a small burden to add a private constructor with
the aid of a code template. The benefit is the elimination of the cost
of pointless code, and a cleaner expression of purpose self-documented
in the code. Potential abuses of the type structure will be caught at
compile time rather than maintenance time, at less cost than the latter,
and are not deployable.

Synergistically, the habit of such conservatisms creates overall benefit
that is greater than the sum of the incremental benefits, at a cost that
is somewhat less than the simple sum due to learning curve effects and
economies of scale. Small benefits have their place in this scheme. If
something is easy, correct and even a little beneficial on balance, why
not include it in the first place, and in the end reap the greater
benefit of a system comprising well-polished, simplified and correct
components?

That was a lot of words.

But I can not see a point in cluttering the code for all
the good and mediocre programmers to prevent the very bad
programmers from writing harmless clutter.

There are lots of more serious problems to tackle.

I am all for restricting programmers ability to shoot
themselves in the foot.

But I do not see the any purpose in restricting them from
hitting themselves in the head with a feather.

Arne
 
A

Arne Vajhøj

I've read all the replies to this thread and have been trying to post
followup questions since yesterday afternoon; unfortunately, I've had
major problems with the news server and have finally had to resort to
posting via Google groups. Sorry for the delay!

--
Am I correct in assuming that a valid example of 'state' is the Locale
used by the class/methods to generate error messages?

My StringUtils class generates a substantial number of error messages
via the standard ResourceBundle mechanism and delivers them in a
Locale-sensitive fashion. I have two constructors for StringUtils, one
that has no parameters and simply uses the default Locale, and the
other, which has the Locale desired by the user as the single
parameter.

Now, if I want to continue to deliver Locale-sensitive messages and if
I make the constructors private, I can't tell the methods which Locale
to use in selecting message files via the constructor. I suppose I
could add a Locale parameter to each method's parameter list but that
seems ugly.

What's the best way to handle Locale-sensitive messages if I want to
keep my methods static and my constructors private?

Or does this factor change everything and now justify keeping the
StringUtils constructors public and its methods non-static?

It does.

And it somewhat illustrates the old fact that very few real
world classes only contain static members.

java.lang.Math is one but it is one of very few.

Arne
 
A

Arne Vajhøj

True. The key here (for me anyway) is "utility methods." Short, well
understood algorithms that are unlikely to change, other than globally.
Example:

Math.cos( double ) ...

It's pretty unlikely you'd ever change the definition of a cosine. The
definition of a cosine is mathematically pretty well understood. If you
did change its implementation, you'd likely want those changes to affect
all code globally.

The interface is the definition. A Math interface would never
change the definition of cos, but the implementations NativeMath
and PortableMath (which is more or less todays Math and StrictMath)
could.

The reason that they don't is not because it does not make sense
from an OO perspective - it is because it would really clutter
mathematical code.

Arne
 
J

John B. Matthews

markspace said:
A lot of what is being discussed here is based on patterns from a
book called "Effective Java" by Joshua Bloch. It's basically "design
patterns for Java" and it's very good. You should pick it up.
Whether you use its ideas or not is up to you, but I think it would
extend your understanding of Java quite a bit.

"Item 1: Consider static factory methods instead of constructors"

<http://www.drdobbs.com/java/208403883>
 
L

Lew

markspace said:
OK, now I think I see what you are asking. "Should this class be made
"final"". My opinion: no. Never never make a class final unless you
have thought about it and are sure it must be final.

There's some extra design required when you DON'T make a class final,
but you should always go through that process. The reason is I think
called The Open-Closed Principle, and basically it means that you should
always leave open the possibility that someone will want to extend your
class. Try to let them if at all possible. Other principles aside
("Prefer composition to extension.") it's pernicious to actually prevent
extension unless you have a good reason. Imo.

I reserve the right to prevent extension of my class.

My bias formally is to make a class 'final', but in practice I rarely do, and
while I don't actually design for inheritance as such, I avoid many pitfalls
by 'final' methods, lots of 'private', and the like. The potential damage is
contained. Reasonable use by subclasses won't do any harm, and unreasonable
use leaves no one but the subclasser to blame.

But I'm ready to declare a class 'final' any time. Yep. Just as soon as it
matters enough. Ayep.
 
M

markspace

Arne said:
The reason that they don't is not because it does not make sense
from an OO perspective - it is because it would really clutter
mathematical code.


Since Rhino (the OP) seems somewhat new to Java, I want to add that
Arne's comment is speculation. I doubt anyone here knows the actual
reason the designers of the Java API chose the static pattern they did.

A class with all static methods is a simple, well-understood pattern,
and in general software designers like simple, well-understood patterns.
What the exact motivating factor was for using it in this case I cannot say.
 
M

markspace

Lew said:
but in practice I rarely
do, and while I don't actually design for inheritance as such, I avoid
many pitfalls by 'final' methods, lots of 'private', and the like. The
potential damage is contained. Reasonable use by subclasses won't do
any harm, and unreasonable use leaves no one but the subclasser to blame.


This is a pretty good description how I design classes too, given the
normal time pressures a software engineer is under. Just quoted for
Rhino's benefit, and in case anyone else wants to comment.
 
A

Arved Sandstrom

markspace said:
This is a pretty good description how I design classes too, given the
normal time pressures a software engineer is under. Just quoted for
Rhino's benefit, and in case anyone else wants to comment.

We had a lengthy thread about final not so long ago, although it never
hurts to have it again.

I'm pretty much with Lew and yourself on this one. I don't normally
write public Java libraries; what I do write are classes that go into
(or already belong to) moderate to large size custom J2EE apps. The
original apps I help maintain and fix up aren't usually well written and
most often contain unnecessary and confusing subclasses already - I'm
not about to encourage adding more. I'll do exactly what Lew does when I
am plugging leaks and tightening yards: I'll privatize methods, make
them final occasionally, and every so often make a class final.

To me the distinction lies between designing and implementing for Java
APIs and implementations for the masses, and doing the same for a
bespoke app. Encouraging (or at least not preventing) sub-classing is OK
for the former situation; it's not so good for the latter, not as a
general philosophy it's not. With a custom app you'd better have it
spelled out in your design in B&W exactly where potential inheritance is
called for, and specifically design it only where it's spec'ed out.

AHS
 
R

Rhino

A lot of what is being discussed here is based on patterns from a book
called "Effective Java" by Joshua Bloch.  It's basically "design
patterns for Java" and it's very good.  You should pick it up.  Whether
you use its ideas or not is up to you, but I think it would extend your
understanding of Java quite a bit.

The basics of the Factory pattern are:

1. A static method that is the Factory.

2. A private-ish class that is produced by the Factory method.

public class StringUtils {

   private StringUtils() {}

   public StringUtils getInstance() {
     return new StringUtils();
   }

}

That's the idea, the full pattern can be more complicated.  The
reasoning behind it is that there are some things you can't do in a
constructor, so the method gives you a change to add additional code
after (or before) the constructor is invoked.  It's basically an
additional layer of encapsulation on top of the constructor.  You should
  look at the Java API and notice that a lot of the new-ish classes use
this kind of Factory pattern.  Calendar, for example, and some of the
new Collections, like EnumSet.

I don't own a copy of Effective Java but I read the article from Dr.
Dobbs Journal that someone else cited and read that. Curiously, Bloch
says that this technique does not correspond exactly with the GoF
Factory design pattern (first paragraph after the Boolean example) but
perhaps that's nitpicking. In any case, I read Bloch's discussion of
this technique and found a few different examples of the Factory
design pattern as well but for the life of me, I'm still not clear on
exactly what you are proposing for my specific case.

The methods in my StringUtils class are all static now and I have made
the two constructors private. I've created a static factory method
along the lines you describe above. In fact, I've created two of them,
one that takes a Locale as a parameter and one that uses the desfault
Locale. The compiler is not complaining about what I've done, which is
a one hurdle overcome. But I'm not clear yet on how to invoke this
static factory method, which makes me wonder if I'm fully
understanding what you're saying.

Here's an excerpt of my code, showing the constructors and the static
factory methods and a few relevant fields:
----------------------------------------------------
public class StringUtils {

/**
* An instance of a Msg Resource Bundle.
*/
private static ResourceBundle locMsg = null;

/**
* The MessageFormat being used by this class.
*/
private static MessageFormat msgFmt = new MessageFormat(""); //
$NON-NLS-1$

/**
* The locale used by this class.
*/
private static Locale locale = null;

/**
* This constructor creates an instance of the class that uses the
default locale.
*/
private StringUtils() {

locale = Locale.getDefault();

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

/**
* This constructor creates an instance of the class that uses a
specified locale.
*
* @param myLocale the locale that should be used by this instance
of the class.
*/
private StringUtils(Locale myLocale) {

locale = myLocale;

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

/**
* This is a static factory method.
*
*/
public StringUtils getInstance() {
return new StringUtils();
}

/**
* This is a static factory method.
*
* @param myLocale the Locale that should be used by this class.
*/
public StringUtils getInstance(Locale myLocale) {
return new StringUtils(myLocale);
}

//Other methods omitted (all of them are static).
}
-------------------------------------------------------------------

Is this what you have in mind?

If yes, how do I invoke StringUtils.getInstance()? I've tried this
(from another class) in order to get an instance that uses the default
Locale:

StringUtils stringUtils = StringUtils.getInstance();

but it generates a compile error because it doesn't like the static
reference to getInstance(). And this gives the exact same error:

StringUtils stringUtils = new StringUtils.getInstance();

Can you (or anyone else reading this thread) kindly clarify what I've
got wrong and how to correct it?
 
J

John B. Matthews

Rhino said:
/**
* This is a static factory method.
*
*/
public StringUtils getInstance() {
return new StringUtils();
}
[...]
Is this what you have in mind?

If yes, how do I invoke StringUtils.getInstance()? I've tried this
(from another class) in order to get an instance that uses the default
Locale:

StringUtils stringUtils = StringUtils.getInstance();

but it generates a compile error because it doesn't like the static
reference to getInstance(). And this gives the exact same error:

StringUtils stringUtils = new StringUtils.getInstance();

Can you (or anyone else reading this thread) kindly clarify what I've
got wrong and how to correct it?

public static StringUtils getInstance() {
return new StringUtils();
}
 
M

markspace

Rhino said:
In fact, I've created two of them,
one that takes a Locale as a parameter and one that uses the desfault
Locale.


Good! That's fine, and good design.

private static ResourceBundle locMsg = null; ^^^^^^
private static MessageFormat msgFmt = new MessageFormat(""); // ^^^^^^
private static Locale locale = null; ^^^^^^
//Other methods omitted (all of them are static).
^^^^^^
Is this what you have in mind?


Not exactly. All those static field should be instances variables, not
static. All of the methods except the factory should be non-static
(instance) methods.

If yes, how do I invoke StringUtils.getInstance()? I've tried this
StringUtils stringUtils = new StringUtils.getInstance();
^^^

As John pointed out you don't call new on a method. Just invoke the
method, remove the new keyword here.
 
R

Rhino

Good!  That's fine, and gooddesign.


               ^^^^^^>     private static MessageFormat msgFmt = new MessageFormat(""); //

               ^^^^^^>     private static Locale locale = null;

               ^^^^^^> //Other methods omitted (all of them are static).

                                            ^^^^^^


Not exactly.  All those static field should be instances variables, not
static.  All of the methods except the factory should be non-static
(instance) methods.




                                     ^^^

As John pointed out you don't call new on a method.  Just invoke the
method, remove the new keyword here.

Agreed. Putting a 'new' in front of the StringUtils.getInstance()
didn't make sense to me either. I just wanted you to know that I'd
tried to "think outside the box" in case the coding was a bit
unusual ;-)

Now that I've put the "static" in the definition of getInstance(), the
invocation of getInstance() compiles fine. So I was right that I still
need the two constructors (both private) and that there need to be two
getInstances, one with no parameters and one that expects the Locale
as a parameter?

I'll start removing the 'static' from the variables to turn them back
into instance variables....

Thanks everyone for your help so far. I think I can see the light at
the end of the tunnel now.....
 
A

Arne Vajhøj

Since Rhino (the OP) seems somewhat new to Java, I want to add that
Arne's comment is speculation. I doubt anyone here knows the actual
reason the designers of the Java API chose the static pattern they did.

It is speculation that it was what the were thinking.

But it is fact that they have two implementation classes.

It is commonly accepted that with two implementations
an interface is good.

And it seems very likely that they would have been swamped
with complaints if they had made the Math function non-static.

So I would say that the designers should have thought about this
aspect.

They could have tossed something at the recycle bin and
said hit=instance miss=static.

Arne
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,780
Messages
2,569,608
Members
45,241
Latest member
Lisa1997

Latest Threads

Top