[OT] Non-abstract factory design pattern versus using constructordirectory

[marlow.andrew]

Unfortunately that it seen way too often.

Arne

My commiserations to you. I must be lucky. It has been quite some
years now since I last came across over-engineered code like this. The
last time was around the time when GoF came out and people used
projects to try out ideas they read in that book. The patterns were
used gratuitously and thus to the detriment of the code. But it
enhanced the resumes of the people that did it. The most overused
pattern of them all was, of course, the singleton. Gratuitous
'factories' came second.

-Andrew Marlow

 

[RedGrittyBrick]

This is a builder pattern. At least, it can be shortened to:

Frubitz furbitz = new FurbitzFactory().name(name).weight(weight).
age(age).build();

The advantage is, if you have lots of constructor arguments, then this
allows the FurbitzFactory to check the proper configuration before
making a new one. I.e., if you forget the name or weight, it can be
checked at runtime. If you have more than three or four parameters,
then it's a good idea because supplying every possible constructor can
be very tedious and error prone.

To amplify this point, sometimes it isn't possible to supply every
possible constructor because they would not have distinct signatures.

Silly example:

class Foo {
private String a,b,c;

Foo(String a, String b, String c) {
this.a=a; this.b=b; this.c=c;
}

Foo(String a, String b) {
this.a=a; this.b=b; this.c="default";
}

Foo(String a, String c) { // Oops, same signature
this.a=a; this.b="default"; this.c=c;
}

Foo(String b, String c) { // Oops, same signature
this.a="default"; this.b=b; this.c=c;
}

Foo(String a) {
this.a=a; this.b="default"; this.c="default";
}

Foo(String b) { // Oops, same signature
this.a="default"; this.b=b; this.c="default";
}

Foo(String c) { // Oops, same signature
this.a="default"; this.b="default"; this.c=c;
}
}
...
new Foo(q,p); // not immediately obvious that this is an error.

The builder pattern starts to look quite attractive to me. Obviously one
can mitigate the above issue by striving to have more interesting
parameters. Perhaps by extending or composing the more commonplace classes.

I must go re-read Bloch (no, the other one) -
Item 1: Consider static factory methods instead of constructors
Item 2: Consider a builder when faced with many constructor
parameters
Item 40: Design method signatures carefully

 

[Tomek Matynia]

A minor point, but factories are probably more future-proof. If you
decide that a class would be, in the future, better designed as a light
hierarchy based on some of the inputs via a factory method, you can make
that change without breaking compatibility. If you have a constructor,
you're stuck with it. This point, though, does need to be evaluated on a
case-by-case basis.

Factories are also flexible in the creation of objects. Suppose you
decide at some point that you need (for performance reasons) to have
interned representations of objects. A constructor has to create a new
object, but a factory can choose to reuse an existing item.

Those two are the first reasons that come to my mind; there are other
reasons, though (probably).

I agree with Joshua, but I'd like to add that you can use factories
generally to hide construction details - especially when construction
takes more than one call (i.e. more than constructor call) then you
DRY. Factories may also 'know how' to create an instance. More - they
can also provide additional parameters to constructor which were not
passed to factory method.

I see it's not the case here, but I don't see OOD as a strict science
so if you decide to use constructors, because you will never have any
of those issues people have wrote here, then use constructors and
argue with pair partners!

Regards,
Thomas Matynia

 

[Mark Space]

But that's not what they are doing. They did not employ the pattern
for that purpose.

Just curious, what purpose do they employ the pattern for? You only
mention one, below.

Agreed. And that was my point.

Sounds like you work at "Java 4 Dummies." Seriously, they want to make
the code so simple to read and use, that they could higher absolutely
anyone to maintain it. This is the only semi-rational explanation I can
think of. Or resume padding, as you mention.

Well, that would *work*, of course, but what's wrong with just using
the ctor to construct the object?

Typo. I went back and edited the note, and mistakenly added "static" in
front of "method" where it wasn't needed. Nothing's wrong with a single
constructor. It's preferable in fact. What's being done now sounds
like gratuitous gold plating.


I'd be interested in *your* analysis of what's going on. We've beat the
technical aspects to death, and they don't add up. So it must be
something else. What are the personal interactions here? Is this
company wide policy? Did you just get the pair partner nobody wants?
Etc.... I think it's time to look beyond the software and investigate
the "peopleware."

 

[Arved Sandstrom]

Hmm. Ask them again at lunch. This sounds a lot to me like "i read that
factories are a good idea, and i'm too stupid to actually think about it,
so i'm going to use them and refuse to explain my decision".

[ SNIP ]

I have to ask you this, Tom, since you are (I believe) an agile proponent.
The situation here seems to be a breakdown of the agile philosophy. If I
have the terminology straight here, a navigator imposed an incorrect (or at
the very least, obscure) design decision on a driver - not once, but twice.
If this is actually the way agile is supposed to work - that the navigator
has the final say - for me that's a showstopper.

To be fair, I don't think agile is supposed to work like this. I would have
thought that the driver, as the active coder, could require the navigator to
explain why they wished to make a change.

AHS

 

[Arne Vajhøj]

My commiserations to you. I must be lucky. It has been quite some
years now since I last came across over-engineered code like this. The
last time was around the time when GoF came out and people used
projects to try out ideas they read in that book. The patterns were
used gratuitously and thus to the detriment of the code. But it
enhanced the resumes of the people that did it. The most overused
pattern of them all was, of course, the singleton. Gratuitous
'factories' came second.

I was not thinking specifically about GoF patterns.

It is a general anti-pattern (!) that when XYZ come
out as a good solution to A then some programmers
apply it to all of A, B and C.

Arne

 

[Tom Anderson]

I have to ask you this, Tom, since you are (I believe) an agile proponent.

I have indeed proponed it in the past, and i suppose it's my duty to step
up again here!

The situation here seems to be a breakdown of the agile philosophy. If I
have the terminology straight here, a navigator imposed an incorrect (or
at the very least, obscure) design decision on a driver - not once, but
twice. If this is actually the way agile is supposed to work - that the
navigator has the final say - for me that's a showstopper.

To be fair, I don't think agile is supposed to work like this. I would
have thought that the driver, as the active coder, could require the
navigator to explain why they wished to make a change.

Firstly, i should say that i don't know what the chapter-and-verse party
line on this is.

My understanding of pair programming has always been that decisions are
made by consensus - if you disagree over the way to do something, you stop
and discuss it until you reach agreement. Having a coding standard stops
you arguing about trivial things like where to put braces. Having and
writing unit tests stops you arguing about whether something will work or
not - you can try it, and find out. The "do the simplest thing that could
possibly work" and "refactor mercilessly" principles mean that you can at
least defer arguing about design - when you disagree on something, pick
the option which is simplest right now, and if you run into trouble
because of it, you can come back and fix it.

However, clearly, there will be times when you have a disagreement on
which you don't reach an honest consensus, and you need a method to
resolve that. I don't think having a rule that the driver always decides,
or that the navigator always decides, is helpful, because who is driver at
any point can be pretty arbitrary. Equally, i think it would be corrosive
to go by seniority and let the ranking programmer make the decision. Any
of those rules would undermine the need for consensus.

I'm trying to think how this had been dealt with when i've been pairing. I
think the key thing is that both members of the pair should approach the
enterprise in good faith: that means being prepared to take time to
explain your ideas to the other person, and to take their criticism on
board, but also being prepared to give your partner's ideas the benefit of
the doubt. And sometimes, swallowing your pride and letting the other guy
have his way even though you think he's wrong - knowing that if he is,
you'll find out soon enough. If you have one or two people who can't do
that, then i don't think there's much hope for them pair programming.

This week, we were tackling a story about having a tool to do some
analysis of JSPs. The first step is to parse the JSP. To me, it was
obvious that we should reuse the parser from an existing JSP engine,
adapting it to our needs; my partner (who is much more senior than me -
and that could mean either more expert or more stuck in his ways, as you
prefer!) told me he thought that was a waste of time, and that we should
hand-write a recursive-descent parser from scratch. We argued briefly,
then he agreed to humour me, and we set about hooking into Jasper's
parser. After half a day, we'd discovered that (a) Jasper's parser is a
complete pain to use outside of the rest of Jasper, and (b) although it
parsed the JSP-specific stuff, it did nothing to help us parse the HTML in
the pages, which we also needed to do. I then admitted that i'd been
wrong, and we adopted the handwritten parser strategy. Another half a day
later, we had a parser that handled enough JSP and HTML to do what we
needed (HTML/JSP actually being a pretty simple syntax). Is there a lesson
here? There was give and take in both directions, and ultimately, it was
the code that made the decision - one approach worked, one didn't.

tom