A little afternoon WTF

Discussion in 'Java' started by Tom Anderson, May 13, 2010.

  1. Tom Anderson

    Tom Anderson Guest

    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

    --
    No gods, no masters.
    Tom Anderson, May 13, 2010
    #1
    1. Advertising

  2. Tom Anderson

    Lew Guest

    Tom Anderson wrote:
    > 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.

    --
    Lew
    Lew, May 13, 2010
    #2
    1. Advertising

  3. In article <>,
    Tom Anderson <> wrote:

    > 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.


    [...]

    --
    John B. Matthews
    trashgod at gmail dot com
    <http://sites.google.com/site/drjohnbmatthews>
    John B. Matthews, May 13, 2010
    #3
  4. Tom Anderson

    Roedy Green Guest

    On Thu, 13 May 2010 18:12:28 +0100, Tom Anderson
    <> wrote, quoted or indirectly quoted someone who
    said :

    > 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.
    Roedy Green, May 13, 2010
    #4
  5. Tom Anderson

    Tom Anderson Guest

    On Thu, 13 May 2010, Lew wrote:

    > Tom Anderson wrote:
    >> Greets yalls,

    >
    > Plural of a plural?


    Word!

    >> 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),


    Eugh, i didn't even see that.

    >> 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.


    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

    --
    everything is temporary
    Tom Anderson, May 13, 2010
    #5
  6. Tom Anderson

    Tom Anderson Guest

    On Thu, 13 May 2010, John B. Matthews wrote:

    > In article <>,
    > Tom Anderson <> wrote:
    >
    >> 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";


    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

    --
    everything is temporary
    Tom Anderson, May 13, 2010
    #6
  7. Tom Anderson

    Tom Anderson Guest

    On Thu, 13 May 2010, Roedy Green wrote:

    > On Thu, 13 May 2010 18:12:28 +0100, Tom Anderson
    > <> wrote, quoted or indirectly quoted someone who
    > said :
    >
    >> 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.


    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

    --
    everything is temporary
    Tom Anderson, May 13, 2010
    #7
  8. Tom Anderson

    Steve Sobol Guest

    In article <>,
    says...
    >
    > 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. :)


    --
    Steve Sobol, Victorville, California, USA
    Steve Sobol, May 13, 2010
    #8
  9. In article <>,
    Tom Anderson <> wrote:

    > On Thu, 13 May 2010, John B. Matthews wrote:
    >
    > > In article <>,
    > > Tom Anderson <> wrote:
    > >
    > >> 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";

    >
    > 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.

    --
    John B. Matthews
    trashgod at gmail dot com
    <http://sites.google.com/site/drjohnbmatthews>
    John B. Matthews, May 13, 2010
    #9
  10. Tom Anderson

    Eric Sosman Guest

    On 5/13/2010 4:45 PM, Robert Klemme wrote:
    > On 13.05.2010 20:13, Roedy Green wrote:
    >>
    >> The "" + idiom is sometimes used to force an int to string. Perhaps
    >> he cloned code from such an example.

    >
    > 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.

    --
    Eric Sosman
    lid
    Eric Sosman, May 13, 2010
    #10
  11. John B. Matthews wrote:

    > Double + is surely ungood.


    Very nicely done.
    Mike Schilling, May 13, 2010
    #11
  12. Robert Klemme wrote:
    > On 13.05.2010 20:13, Roedy Green wrote:
    >> On Thu, 13 May 2010 18:12:28 +0100, Tom Anderson
    >> <> wrote, quoted or indirectly quoted someone
    >> who said :
    >>
    >>> 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.

    >
    > 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.
    Mike Schilling, May 13, 2010
    #12
  13. Tom Anderson wrote:
    > 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
    Arved Sandstrom, May 13, 2010
    #13
  14. 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";

    --
    Stanimir
    Stanimir Stamenkov, May 13, 2010
    #14
  15. Tom Anderson

    Lew Guest

    John B. Matthews wrote:
    > 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.

    --
    Lew
    Lew, May 14, 2010
    #15
  16. Hi!

    Tom Anderson wrote:

    > .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.
    Gunter Herrmann, May 14, 2010
    #16
  17. In article <hshr86$ebe$-september.org>,
    Stanimir Stamenkov <> wrote:

    > 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";


    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 "".

    --
    John B. Matthews
    trashgod at gmail dot com
    <http://sites.google.com/site/drjohnbmatthews>
    John B. Matthews, May 14, 2010
    #17
  18. Tom Anderson

    markspace Guest

    Wayne wrote:

    > On 5/13/2010 1:12 PM, Tom Anderson wrote:
    >> "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" +
    >> "<initech:tps-report><initech:coversheet> etc";


    >
    > 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.
    markspace, May 14, 2010
    #18
  19. markspace wrote:
    > Wayne wrote:
    >
    >> On 5/13/2010 1:12 PM, Tom Anderson wrote:
    >>> "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" +
    >>> "<initech:tps-report><initech:coversheet> etc";

    >
    >>
    >> 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.


    That is, that LF, CR, and CRLF are all translated to simple LF by a
    conforming XML processor.
    Mike Schilling, May 14, 2010
    #19
  20. "Mike Schilling" <> writes:
    > Robert Klemme wrote:


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


    > 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

    --
    Jukka Lahtinen
    Jukka Lahtinen, May 14, 2010
    #20
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Joe Van Meer

    wtf am I doing wrong?

    Joe Van Meer, May 6, 2004, in forum: ASP .Net
    Replies:
    2
    Views:
    363
    Joe Van Meer
    May 6, 2004
  2. Stylus Studio

    [announce] An Afternoon of XML !!

    Stylus Studio, Sep 9, 2006, in forum: XML
    Replies:
    0
    Views:
    329
    Stylus Studio
    Sep 9, 2006
  3. ThaDoctor
    Replies:
    3
    Views:
    372
    Alan Woodland
    Sep 28, 2007
  4. Cameron Simpson

    this afternoon's duck typing exercise ...

    Cameron Simpson, Mar 2, 2012, in forum: Python
    Replies:
    1
    Views:
    173
    Steven D'Aprano
    Mar 2, 2012
  5. Daniel
    Replies:
    1
    Views:
    200
    Bart van Ingen Schenau
    Jul 9, 2013
Loading...

Share This Page