• Brian (unregistered)

    Actually that looks like "good" .NET code. MSFT says using the f.Append with the String Builder is faster than doing a simple:

    TYPE_OF_LOCATION & " = " & this.HomeLocationCode.ToString()

    I don't like it but it seems.

  • Jim Bolla (unregistered)

    perhaps the " key on the developer's keyboard was broken and it was easier to define them all as constants instead of copy/pasting it in there every time it was needed. :)

  • cooper (unregistered)

    Its not that he is using StringBuilder (actually in Java it is the same recommendation with StringBuffer), its that somehow " " and "=" are stored as contants.


    NOW....

    I will freely admit to doing some weird looking stuff like that in Java occationally for a good reason: If you reference fixed (even simple) String contants as static instance references, you can evaluate constant expressions using == and other reference equality systems (Vector.contains, etc).

  • Anders Engström (unregistered)

    Um.. SQL-injection warning. The correct should of course be:

    StringBuilder f = new StringBuilder();
    f.Append(TYPE_OF_LOCATION);
    f.Append(PConstants.SPACE);
    f.Append(PConstants.EQUALS);
    f.Append(PConstants.SPACE);
    f.Append(PConstants.QUESTIONMARK);
    return f.ToString();

    Then populate a PreparedStatement (or whatever it's called in .NET).

  • cooper (unregistered)

    >> Um.. SQL-injection warning.

    I think he's to-Stringing an Integer value of a PK field on a lookup table. No threat there.

    Though I have to constantly beat people over the head at my job about not using PreparedStatements.

  • fogelman (unregistered)

    I don't buy the .Append deal. If one is that worried about speed that he's using StringBuilder to avoid repetitive string catenation, how come he is not worried about DB compiling the ad hoc query? and since SB can take a string to append, he should have done a " = " in one assignment .. blah.

  • Søren Lund (unregistered)

    Doesn't it smell a bit of some misguided try to mimick the Environment.NewLine and the likes constants?

  • Mike Dimmick (unregistered)

    No-one reads Rico Mariani (blogs.msdn.com/ricom) then?

    The C# and VB.NET compilers both turn TYPE_OF_LOCATION & " = " & this.HomeLocationCode.ToString() ('+' not '&' for C# developers) into a call to String.Concat(string, string, string), which is quicker than StringBuilder.Append - it knows how much memory to allocate, the sum of the lengths of the three strings.

  • Ming Chen (unregistered)

    I can think of couple case it will make perfect sense.

    a. in nText column instead SQL will only take the "LIKE" in the where clause.


    b. Sometime the Where should be "IS NULL" when this.HomeLocationCode.ToString()) happened to return NULL. And As far as I know " = NULL" is not legal SQL.

    The question become can those "constant" been remap to "LIKE" or "IS".

    Without providing the context for the program, I am not sure who is laughing at who. (ie, is one laugh at one for lacking of SQL knowledge or the other laugh at one on lacking of C# knowledge....)

  • Addy Santo (unregistered)

    Just to clarify:

    1. Performance wasn't the issue. If your application is too slow, I seriously doubt that string concating of a single SQL WHERE clause is what will save you.

    2. Ming: The scenarios you described are far beyond anything those developers could imagine.

    Just believe me that the code snippet was someone's sincere best effort at creating a "location=1" string...

  • Delphi7 (unregistered)

    I have seen this before - Junior developers were given guidence documents that state there should be no "magic values". And they take it litteraly and religously.

  • Scott (unregistered)

    I can vaguely understand someone mandating a rule that you have no string constants used anywhere, except as statics defined beforehand. It's the kind of rule that comes from NLS'ing everything.

  • Cablito (unregistered)

    lol

  • Delphi7 (unregistered)

    Yeah but in that case - even property names were considered "constants" and they acceses all properties using reflection (or they didn't use properties at all - they had one big hashmap where they dumped all of it in. (this was a java project))

  • Jeff Darcy (unregistered)

    Maybe it was an attempt at internationalization. You never know when the governor of Tajikistan's coastal provinces might demand that people use at-signs instead of spaces, and this code is fully prepared for that eventuality.

Leave a comment on “Just in case SPACE isn't supported in the future”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article