A little afternoon WTF

[Tom Anderson]

Greets yalls,

For your edutainment, some code (lightly anonymised) seen while digging
into code written by some (now-departed) contractors today:

private static String header = "" +
"<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" +
"<initech:tps-report><initech:coversheet> etc";

WTF: the empty string literal on the first line. What did they think that
was for?

Secondary WTF: writing XML by hand rather than using StAX or something.
That i'm less outraged about, because it's ordinary ignorance, rather than
the special kind of brainwrong reflected in the primary WTF.

Oh god, i've just spotted another one: the hardcoded CRLF! This is a
linux-only project (up to and including developing on linux VMs - the only
time you'd ever look at this file would be on a linux machine), and XML
normalises all line breaks to LF anyway. Why would you do that?

Still, at least it's not this, from the other end of the project:

private static final String TEST_DATA_QUERY = new StringBuilder().append(" select * ")
.append(" from tps_report, tps_folder where tps_folder.id=tps_report.id ")
.append(" and tps_folder.id=2057 ").toString();

I've got no beef with the SQL, that's fine, but WTF: the explicit
StringBuildering. The surplus spaces at the end of the strings and the
lack of indentation on the append lines are the icing on the cake. The way
the where clause is half on the same line as the from and half not is good
too.

None of this is strictly incorrect - it all works - but there's something
distinctly *wrong* about it. It doesn't fill you with confidence that the
important things are done correctly. Nor does actually looking at the
important things, as it happens, because one immediately sees that they
aren't.

tom

 

[Lew]

Greets yalls,

Plural of a plural?

For your edutainment, some code (lightly anonymised) seen while digging
into code written by some (now-departed) contractors today:

private static String header = "" +
"<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" +
"<initech:tps-report><initech:coversheet> etc";

WTF: the empty string literal on the first line. What did they think
that was for?

Secondary WTF: writing XML by hand rather than using StAX or something.
That i'm less outraged about, because it's ordinary ignorance, rather
than the special kind of brainwrong reflected in the primary WTF.

Not to mention the lack of 'final' on the variable (and concomitant consequent
violation of the naming conventions),

Oh god, i've just spotted another one: the hardcoded CRLF! This is a
linux-only project (up to and including developing on linux VMs - the
only time you'd ever look at this file would be on a linux machine), and
XML normalises all line breaks to LF anyway. Why would you do that? ....
None of this is strictly incorrect - it all works -
but there's something distinctly *wrong* about it.
It doesn't fill you with confidence that the important things are done correctly.
Nor does actually looking at the important things, as it happens, because
one immediately sees that they aren't.

You raise a very important point, perhaps several. Mainly, when a coder is
that careless and stupid in the (seemingly) unimportant details, odds are that
they're that careless and stupid everywhere. Conversely, those who wish to be
excellent coders should not be careless and stupid anywhere.

 

[John B. Matthews]

Greets yalls,

For your edutainment, some code (lightly anonymised) seen while digging
into code written by some (now-departed) contractors today:

private static String header = "" +
"<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" +
"<initech:tps-report><initech:coversheet> etc";

WTF: the empty string literal on the first line. What did they think that
was for?

Preferring to break before an operator, I might write this for
readability:

private static final String header = "" +
+ "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ "<initech:tps-report><initech:coversheet> etc";

[...]

Still, at least it's not this, from the other end of the project:

private static final String TEST_DATA_QUERY = new StringBuilder()...

This looks like an inexperienced person was told to use StringBuilder
without realizing the compiler would do it. I might prefer:

private static final String TEST_DATA_QUERY = ""
+ "select * "
+ "from tps_report, tps_folder "
+ "where tps_folder.id = tps_report.id "
+ " and tps_folder.id = 2057";

I've got no beef with the SQL, that's fine, but WTF: the explicit
StringBuildering. The surplus spaces at the end of the strings and
the lack of indentation on the append lines are the icing on the
cake. The way the where clause is half on the same line as the from
and half not is good too.

[...]

 

[Roedy Green]

private static String header = "" +
"<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" +

motive? Contractor may have known precisely what he needed the first
line to look like, but could not bludgeon his XML package to produce
it so gave up and did it manually.

The "" + idiom is sometimes used to force an int to string. Perhaps
he cloned code from such an example.

Using a StringBuilder with precise size estimate I discovered speeded
my static macro code up by 10%. I suspect then that any sort of
framework would similarly benefit. However, he did not specify an
estimate, thus defeating the point of manual StringBuilders.
--
Roedy Green Canadian Mind Products
http://mindprod.com

Beauty is our business.
~ Edsger Wybe Dijkstra (born: 1930-05-11 died: 2002-08-06 at age: 72)

Referring to computer science.

 

[Tom Anderson]

Plural of a plural?
Word!


Not to mention the lack of 'final' on the variable (and concomitant
consequent violation of the naming conventions),

Eugh, i didn't even see that.

You raise a very important point, perhaps several. Mainly, when a coder
is that careless and stupid in the (seemingly) unimportant details, odds
are that they're that careless and stupid everywhere. Conversely, those
who wish to be excellent coders should not be careless and stupid
anywhere.

That's very much how i see it too. It's a matter of professionalism - a
professional takes pride in getting the little details of their work
right, even when they're inconsequential.

tom

 

[Tom Anderson]

Preferring to break before an operator, I might write this for
readability:

private static final String header = "" +
+ "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ "<initech:tps-report><initech:coversheet> etc";

You probably wouldn't, because that has two +s in a row after the empty
string. I assume you would lose the first one, leaving one at the start of
the line. Even with the insistence on operators at the start of the line
(which i do quite like myself), i'd prefer this:

private static final String header
= "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ "<initech:tps-report><initech:coversheet> etc";

tom

 

[Tom Anderson]

motive? Contractor may have known precisely what he needed the first
line to look like, but could not bludgeon his XML package to produce it
so gave up and did it manually.

What XML package? I'm not sure what you mean.

The "" + idiom is sometimes used to force an int to string. Perhaps he
cloned code from such an example.

Possible, although it seems a bit of a stretch. There are string constants
all over that bit of the codebase, and i can't think of a single instance
of the ""+int trick in the entire system. I think it's pretty bad
practice, so i'd remember if i'd seen it. It's quite possible he learned
the idiom from that use, though.

Using a StringBuilder with precise size estimate I discovered speeded my
static macro code up by 10%. I suspect then that any sort of framework
would similarly benefit. However, he did not specify an estimate, thus
defeating the point of manual StringBuilders.

Indeed. And using that approach to initialise static constants whose
values would otherwise be compile-time constants, and so could be stored
complete in the constant pool, would be foolish anyway.

tom

 

[Steve Sobol]

Greets yalls,

For your edutainment, some code (lightly anonymised) seen while digging
into code written by some (now-departed) contractors today:

private static String header = "" +
"<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" +
"<initech:tps-report><initech:coversheet> etc";

WTF: the empty string literal on the first line. What did they think that
was for?

Eclipse inserts stuff like that occasionally when I'm editing multi-line
declarations or statements like that. Difference is, I usually edit it
back out.

 

[John B. Matthews]

You probably wouldn't, because that has two +s in a row after the empty
string. I assume you would lose the first one, leaving one at the start of
the line. Even with the insistence on operators at the start of the line
(which i do quite like myself), i'd prefer this:

private static final String header
= "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ "<initech:tps-report><initech:coversheet> etc";

Much better! Double + is surely ungood.

 

[Eric Sosman]

Roedy, for that String.valueOf(int) is the proper idiom - not "" + int.

Yes, but ""+int is in fact seen all over the place, just
like graffiti, infomercials, and pop-unders.

That's ridiculous.

Yes, but it's reality.

 

[Mike Schilling]

Double + is surely ungood.

Very nicely done.

 

[Mike Schilling]

Roedy, for that String.valueOf(int) is the proper idiom - not "" +
int.

Or int + String:

System.out.println(i + "is the answer");

But people who don't realize that that works will often do

System.out.println("" + i + "is the answer");

just to be sure. It's at most a venial sin.

 

[Arved Sandstrom]

Greets yalls,

For your edutainment, some code (lightly anonymised) seen while digging
into code written by some (now-departed) contractors today:

private static String header = "" +
"<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" +
"<initech:tps-report><initech:coversheet> etc";

WTF: the empty string literal on the first line. What did they think
that was for?

[ SNIP ]

I've seen this as well. And where it shows up is similar to your
example; there's no way that refactoring converted something half
sensible to something...umm...insensible?

Still, at least it's not this, from the other end of the project:

private static final String TEST_DATA_QUERY = new
StringBuilder().append(" select * ")
.append(" from tps_report, tps_folder where
tps_folder.id=tps_report.id ")
.append(" and tps_folder.id=2057 ").toString();

I've got no beef with the SQL, that's fine, but WTF: the explicit
StringBuildering. The surplus spaces at the end of the strings and the
lack of indentation on the append lines are the icing on the cake. The
way the where clause is half on the same line as the from and half not
is good too.

Well, I don't know if I can consider the SQL to be fine, not if you
include the hardcoded "id" value of 2057. I can see that it's evidently
test data but that still makes me queasy.

None of this is strictly incorrect - it all works - but there's
something distinctly *wrong* about it. It doesn't fill you with
confidence that the important things are done correctly. Nor does
actually looking at the important things, as it happens, because one
immediately sees that they aren't.

tom

A lot of this boils down to readability. Java ain't C++, there's no
reason why 99 percent of Java code shouldn't be anything but clear and
comprehensible. If it's not then you've got reason to suspect not just
sloppy layout but sloppy thinking.

AHS

 

[Stanimir Stamenkov]

Thu, 13 May 2010 19:45:40 +0100, /Tom Anderson/:

i'd prefer this:

private static final String header
= "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ "<initech:tps-report><initech:coversheet> etc";

I always break after the assignment operator (I never put it on a
new line):

private static final String HEADER =
"<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ "<initech:tps-report><initech:coversheet> etc";

 

[Lew]

This looks like an inexperienced person was told to use StringBuilder
without realizing the compiler would do it. I might prefer:

private static final String TEST_DATA_QUERY = ""
+ "select * "
+ "from tps_report, tps_folder "
+ "where tps_folder.id = tps_report.id "
+ " and tps_folder.id = 2057";

In this case there is no internal use of StringBuilder, at least not at run
time, as the string is a compile-time constant.

The initial "" is still objectionable.

 

[Gunter Herrmann]

Hi!

.append(" and tps_folder.id=2057 ").toString();

I like magic numbers - not really. And if you really have to do
something like this create a final self documenting static variable with
a talking name.

Best regards

Gunter in Orlando, Fla.

 

[John B. Matthews]

Thu, 13 May 2010 19:45:40 +0100, /Tom Anderson/:


I always break after the assignment operator (I never put it on a
new line):

private static final String HEADER =
"<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ "<initech:tps-report><initech:coversheet> etc";

I was influenced by the "Break before an operator" guideline:

<http://java.sun.com/docs/codeconv/html/CodeConventions.doc3.html>

Tom's leading "=" is appealing when a string literal has a meaningful
indentation of its own. It's urely more appealing than a leading "".

 

[markspace]

Linux or not, text files sent across the Internet must have lines
terminated with CRLF, as per the IETF. See
<http://www.rfc-editor.org/EOLstory.txt>. (I've been bitten by that more
than once, with PHP or JavaScript creating text that browsers subsequently
refuse to read with just a LF.)

Horse pucky. It's XML, which is UTF-8 encoded by default, and also
explicitly in this example. So it's not even ASCII. And the XML spec
says all XML uses new lines only. Raw carriage returns are translated
to newlines if they are encountered in the input text.

<http://www.w3.org/TR/xml/#sec-line-ends>

If you want to quote specifications, be sure to quote the correct one.

 

[Mike Schilling]

Horse pucky. It's XML, which is UTF-8 encoded by default, and also
explicitly in this example. So it's not even ASCII. And the XML
spec says all XML uses new lines only.

That is, that LF, CR, and CRLF are all translated to simple LF by a
conforming XML processor.

 

[Jukka Lahtinen]

Robert Klemme wrote:

Or int + String:
System.out.println(i + "is the answer");

42is the answer

I would prefer having a space between the int and the rest of the
String, for readability of the output:

System.out.println(i + " is the answer");

42 is the answer