• (nodebb)

    Just like they "used" StringBuilder, they "programmed" in C#. Oh and BTW, if the operands are known ahead of time, concatenation gets simplified into a string.Concat call, and StringBuilder isn't really needed.

  • Anon (unregistered) in reply to Mr. TA

    Still, they concatenated the output of String.Format instead of using a single String.Format.

    And those freaking Convert.ToDouble as well.

  • Naomi (unregistered)

    PSA: In Java, at least, the advise to use a StringBuilder no longer holds! These days, the + usually compiles to StringBuilder operations, but the compiler and even the runtime can usually optimize them by setting the buffer size intelligently. If you use a StringBuilder manually it's taken as a sign that you know something the compiler doesn't and no optimizations happen. (And of course, the concatenation of constants happens at compile time; the same isn't true for calling append!)

    The one exception is if you're concatenating in a loop, in which case it's still better to create a StringBuilder outside the loop.

  • Brian (unregistered)
    They were transitioning from VB6 to VB.NET and didn't trust developers to adapt to this new world, so they positioned code reviews as a "gateway" and the reviewers were guards, tasked with ensuring that any code going past met the standards. That was already a pretty bad, and pretty hostile approach.

    Adding a code review process when doing a major rewrite is a bad thing? Provided the reviewers are competent and timely, and the focus of the review is on the code, not the coder, it's a useful way to improve the quality of both the code and those writing it. I know I've learned plenty during code reviews - sometimes even as the reviewer.

    Then some code would get submitted which didn't just violate the standards ... But it was mission critical and had a scheduled release date already, so the code review process had to let it pass.

    Ah, there's the WTF. Implement a new process and then ignore it whenever it's inconvenient.

  • LongTimeLurker (unregistered)

    Signs of multiple contributors to this single line of code? string.Format and then later String.Format ...

  • Naomi (unregistered) in reply to Brian

    Adding a code review process when doing a major rewrite is a bad thing? Provided the reviewers are competent and timely, and the focus of the review is on the code, not the coder, it's a useful way to improve the quality of both the code and those writing it. I know I've learned plenty during code reviews - sometimes even as the reviewer. To me, it sounds like the framing of the process at this company was unnecessarily adversarial. It's like you said, a code reviewer and their reviewee should be working together. If the company framed it as an aggressive process, like Remy's description implied? If you'll pardon the expression, that's TRWTF.

  • Naomi (unregistered)

    Belay that; TRWTF is trying to write Markdown before I've had my coffee.

  • Doug (unregistered)

    TRWTF is the lack of a Preview feature.

  • SyntaxError (unregistered)

    No easy reader version today, Remy?

  • sizer99 (google) in reply to Anon

    I'm guessing the Convert.ToDouble is because headerAmt is dollars and cents stored in an int, and if you just do headerAMT/100 you're doing integer division and the cents get truncated.

    Of course there are easier to read ways to do that like (double) headerAmt / 100 or headerAmt / 100.0, and the correct way to do this is to store headerAmt in a Decimal type which is specifically designed to do this. But once you're stuck with cents in an int you do need to coerce it, or calculate the dollars and cents separately (div then mod).

  • Anon (unregistered) in reply to sizer99

    If it is stored as cents in an int then it is super easy: String.Format("{0:0\.00}", headerAmt) as long localization isn't an issue.

    In any other weird format, Convert.ToDouble usually is an overkill, a static implicit or explicit conversion usually is preferable.

  • WTFGuy (unregistered)

    Brace yourself for attempted Markdown

    Remember this is VB, not C#. in C#

    int headerAMT = whatever // value in cents  
var result = (double)headerAmt / 100 // the calc implicitly widens the integer 100 into a double, does division on the two doubles, and result is a double containing dollars as the integer part and cents as the fractional part

    in VB the corresponding vernacular is

    Dim headerAMT as Integer = whatever  ' value in cents  
result = CDbl(headerAMT) / 100 ' result is a double containing integer dollars and fractional cents

    In VB.Net, CDbl() is the language-preferred vernacular which is almost, but not quite exactly, equivalent to .Net's language neutral Convert.ToDouble(). See https://docs.microsoft.com/en-us/dotnet/visual-basic/language-reference/functions/type-conversion-functions under "Remarks" if you're skeptical.

    Explicitly declaring the target type to be double is no help in and of itself; the division between two integers will still be an integer.

    At least to my stylistic eye, doing the explicit cast (double) or CDbl() is a lot more obvious & non-tricky than doing someVarNameThatHappensToBeOfTypeInt / 100.0 and expecting the next dev reading this code to recognize that the ".0" is driving an implicit cast of a different value in the expression. Even doing someVarNameThatHappensToBeOfTypeInt / 100R or .../100#would be better than just the unadorned ".0" .

    Then again, I'm used to reading shitty code by folks who never think of these corner case or type issues, so I'm sorta spring loaded to write code that loudly tells the next dev "Yes, I thought of that. And here's my mitigation in plain sight."

  • Old timer (unregistered)

    TTWF was building a compiler that required a String Builder method to do efficient concatenation.

    C as originally conceived did not have a string type: it used character arrays. And had to use functions because they were supplied by the library, not by the language. But Java was a product of the last decade of the 20th century. and C# was a product of the 21st century! Languages with string objects and efficient concatenation had been around for decades. The decision to not include efficient string handling in the language was an example of willful blindness, NIH syndrome, and deliberate obtuseness.

    Ah well. Classic computer programmer vanity.

  • David Mårtensson (unregistered) in reply to Old timer

    The reason Java and C# both uses string builder is because both wanted to use immutable strings to avoid problems with multi threaded code that plague solutions that allow changing of strings.

    Mutable strings would prevent a lot of performance optimizations and introduce lots of locking, especially if the compiler should handle it.

    Over time, both Java and C# has made improvements where the compiler analyses more and more of the usage to "fix" bad usage but it can never make up for good code from the developer.

    If you really want to get around all this you have to go to a whole different programming style and that scares most people of ;)

    StringBuilders are not always the answer, but it is and was good enough when they created the languages.

  • Tim (unregistered)

    I can't remember the last time I used a StringBuilder in C#. What with string interpolation and compiler optimization of concatenation, it's probably only useful in a loop and even then, string.Join is usually preferable.

    Importantly all of these allow immutability whereas StringBuilder is all about mutating state which can make the intent of the program more difficult to understand, so there would have to be a compelling performance argument for me to use a StringBuilder nowadays.

  • SG (unregistered) in reply to Naomi

    In Java, at least, the advise to use a StringBuilder no longer holds! These days...

    And when you say, "these days", you mean for the last fifteen years... that particular optimisation came in with Java 5. StringBuilder should still be used when building up complex strings with a lot of conditional or iterative logic, but it's been a very very long time since there was any justification for using it elsewhere.

  • (nodebb) in reply to SG

    Java 5? Back when we only had StringBuffer, javac was happily using it for multiple concatenations. It's too far back for me to remember which version of Java this was, but I suspect no later than Java 2.

  • Peter Wolff (unregistered) in reply to Mr. TA

    Actually, String.Concat implicitly calls StringBuilder (or one of its base classes, maybe).

  • (nodebb)

    (The real WTF is that I need to log in twice every time; there's an error the first time.)

    ((But the actual real WTF is that, without StringBuilder, the code is 43% faster and uses a little over a third the memory.))

    Turns out cargo-cult programming isn't just stupid, but also slow. https://gist.github.com/chucker/1ec48f7923423b1d9bc5da2d78c29f34

  • Craig (unregistered) in reply to WTFGuy

    In VB, the conversion gymnastics are unnecessary. Unlike C-heritage languages, VB doesn't overload the division operator. A forward slash is floating point division, a backward slash is integer division. result = headerAMT / 100 will give you a floating point result even if headerAMT is an integer.

  • (nodebb) in reply to chucker23n

    It would be interesting to see what byte code gets generated in each case.

Leave a comment on “The Standard StringBuilder”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article