

Java memes which refuse to die - iliastsagklis
http://www.javacodegeeks.com/2012/08/java-memes-which-refuse-to-die.html

======
jonhohle
Nearly all of the times I've seen StringBuilder used incode reviews, using the
+ operator or a better design would have been more appropriate. There are a
few cases (like looping over a collection), where StringBuilders make sense,
but usually they are needlessly verbose.

~~~
nl
StringBuilders are verbose, but they _do_ make a measurable difference in your
code.

The problem with no using them is that then you need to be sure your code is
never called by something else that runs it in a loop.

A fair while ago (JDK 1.5 had just been released, so I was still looking at
StringBuffer) I did some analysis on the bytecode generated by concatenation.

There is a pretty significant difference:

[http://nicklothian.com/blog/2005/06/09/on-java-string-
concat...](http://nicklothian.com/blog/2005/06/09/on-java-string-
concatenation/)

~~~
jonhohle
The example in that article is fairly dumb (why would you use concatenation as
an input to a StringBuffer). The Java compiler converts concatenation to
StringBuilder in Java 1.6. In most cases (eight the exception of loops and
federated, organized string creation) the + operator is identical in
performance to StringBuilder (because that's what it compiles to).

------
dkhenry
There are a lot more then 4. I didn't even see anything about Factory
Factories.

~~~
mcherm
Factory factories USED to be a problem in my Java programming, but they aren't
anymore because I created a configurable builder class for generating my
factory factories. I call it FactoryFactoryBuilder, and it completely
eliminated the factory factory problem.

~~~
dkhenry
That sounds like a great idea. I bet you could even use some XML configuration
files to make it even more extensible.

------
lukev
I would add boilerplate getters/setters to the list. Almost always, a simple
public field works just fine.

~~~
delinka
Using a public field removes one of the major benefits of object-oriented
design. Don't misunderstand me: in many cases, really all you need is a
collection of related fields and making them public is indeed OK - like a
struct in C.

But I tend to err on the side of safety with my fields - private member,
public accessors - while I work out my modeling and refactoring. When the
design settles, then I can relax access if it's OK. And when something changes
in the design (atomicity requirements, immutability, etc) I already have most
of the boilerplate in place. Mostly though, I keep my accessors.

On a related note, do Java IDEs not provide boilerplate code? I'm more an
Objective-C guy and the language now supports "here's a property" and "create
the accessors" alongside my needs for custom ivar names, custom setters, etc.
Even if Java the language doesn't have similar features, I'd expect IDEs to
give you this kind of boilerplate stuff.

~~~
lukev
They do allow code generation, and that's precisely the problem. You end up
with these ridiculously bloated files, most of the code of which hasn't ever
even been looked at by a human.

But generating getters and setters based on fields is putting the cart before
the horse. You're right, it is very important for your classes to have a
stable, clean public API that's distinct from the private implementation. But
if that public API is intended to expose data at the field level, then just
expose the fields without confusing the issue. If you don't want to expose the
fields, then don't.

Having private-but-not-really fields with a lot of auto-generated getters and
setters doesn't help you provide a clean, small API: instead, it _bloats_ the
public API. Also, it obfuscates what's actually happening... is a getter just
a simple data value, or is it doing a bunch of complicated shit behind the
scenes? And if it _is_ doing a bunch of complicated shit, just make it a
regular method on the public API, not a getter that's logically tied to a
field that the client knows is there.

TL;DR: Fields are fields, methods are methods, auto-generating getters/setters
blurs the line in unhelpful ways.

~~~
jpatte
_if that public API is intended to expose data at the field level_

> That doesn't make a lot of sense. An API is an interface, which in this case
> exposes data. The fact that this data is held by objects at the field level
> as nothing to do with the purpose of the API: it's an implementation
> concern. The consumers of this API do not, and should not, need to know
> whether a field or anything else is used to provide the requested data. If
> the public API gets bloated when using getters/setters then blame Java, not
> this basic encapsulation principle.

 _is a getter just a simple data value, or is it doing a bunch of complicated
shit behind the scenes?_

> As you said, and as a rule of thumb, never do a bunch of complicated shit
> inside a getter: always expose a method for this operation. Just because
> getters exist does not mean methods should disappear.

~~~
lukev
_An API is an interface, which in this case exposes data._

Yes. The confusion comes from complecting[1] an object api and data. In
general, having a class that is both a data structure (a bean or a struct) and
a service class is a bad idea.

My contention is that if a class is Just Data, exposing public fields is fine.
And if it is a service class that needs a consistent, stable API, it shouldn't
be exposing fields via getters and setters either.

[1]<http://www.infoq.com/presentations/Simple-Made-Easy>

------
djb_hackernews
The original without all of the noise:

[http://vanillajava.blogspot.gr/2012/08/java-memes-which-
refu...](http://vanillajava.blogspot.gr/2012/08/java-memes-which-refuse-to-
die.html)

------
spacemanaki
Can someone with a better understanding of the Java memory model comment on
this, referring to StringBuffer:

" _Say you had the code_

    
    
      // run in two threads
      sb.append(key).append("=").append(value).append(", ");
    

_Each append is thread safe, but the lock could be release at any point
meaning you could get_ [1]

    
    
      key1=value1, key2=value2,
      key1=key2value1=, value2,
      key1key2==value1value2, ,
    

_What makes it worse is that the JIT and JVM will attempt to hold onto the
lock between calls in the interests of efficiency._ [2]"

[1] is weirdly worded, or even wrong (I don't think "the lock could be release
at any point" is correct) but the sample result does make sense to me if there
are multiple threads banging on it at once. StringBuffer's append methods are
synchronized, but without external synchronization, multiple calls could be
interleaved.

I don't recall ever having heard of [2] before, and my copy of Java
Concurrency in Practice isn't immediately at hand.

~~~
AYBABTME
I don't know if that's the answer you're looking for, but a call to append is
synchronized:

    
    
        public synchronized StringBuffer append(/* Something */);
    

... but chaining them doesn't mean that the chain is synchronized.

    
    
       sb.append(key).append("=").append(value).append(", ");
    

is executed the same way as

    
    
       sb.append(key);
       sb.append("=");
       sb.append(value);
       sb.append(", ");
    

Two concurrent threads running this part of code might do this:

    
    
       sb.append(key);   // # 1
       sb.append("=");   // # 2
       sb.append(value); // # 3
       sb.append(", ");  // # 4
    
       // Thread 1 executes #1
       // Thread 2 executes #1
       // Thread 1 executes #2
       // Thread 1 executes #3
       // Thread 2 executes #2
       // Thread 2 executes #3
       // Thread 2 executes #4
       // Thread 1 executes #4
    

This can be executed in many more different ways.

To ensure that only one thread executes those `append` at once, you would have
to synchronize on the `StringBuffer` instance:

    
    
        synchronize(sb){
            sb.append(key).append("=").append(value).append(", ");
        }
    

Then only one thread will do the appending at a time. But this introduce yet
another lock in your code, and render the `synchronized` methods useless and
redondant.

~~~
spacemanaki
Thanks, I looked up the source and saw that append was synchronized too. I was
mostly asking about this claim from the OP: "the JIT and JVM will attempt to
hold onto the lock between calls in the interests of efficiency". I don't
think that's true.

------
esrauch
It seems bizarre that there are a lot of equivalent classes that are thread
safe or not with names that are simply synonymous. Is it just me?

~~~
spacemanaki
It seems bizarre, but it's just because they were part of different versions.
StringBuffer was in Java 1.0, and StringBuilder was added in Java 1.5. If they
were both part of the original Java libraries they would hopefully have been
given better names. I suppose they could have called StringBuilder
"UnSynchronizedStringBuffer" or "NotThreadSafeStringBuffer", but those are
problematic on their own.

You have the same problems with Vector and ArrayList I guess, although since
ArrayList was added as part of the collections libraries it makes a bit more
sense.

------
surement
s/which/that/g

