• (cs)

    At least they indented it so it quickly stands out that they are retarded.

  • (unregistered)

    At least it looks pretty

  • (unregistered)

    Am I missing something? [*-)] What's wrong with this?

  • (cs) in reply to
    :
    Am I missing something? What's wrong with this?


    Sorry, I forgot to login. That comment is mine.
  • (cs) in reply to

    I've never used J# but I would say that creating a new StringBuffer object everytime, instead of creating it once at the top of the method and using the .append on that one instance of the StringBuffer, is the WTF.

  • (unregistered)

    Wow.

  • (cs)

    Even Visual Bucky would have figured this one out. I agree. Wow.

  • (cs) in reply to haveworld

    @Haveworld:

    Think of this as creating a new rich text item on a new back-end document and using AppendRTItem to add to the string in the real RT field on the real document rather than just using AppendText to build the field as you go. It's pretty much the same in terms of creating and destroying objects, and every bit as unnecessary. Weird.

  • (cs)

    In the past I have smiled when coming across something like:

        StringBuilder sb = new StringBuilder();
        sb.Append("some string" + "more" + "and so on");

    but creating a brand new stringbuilder each time is ingenious. Well done whoever!

  • (cs)

    For those who don't get what's wrong with the code, consider this:

    [code language="c#"]
    public override string getClassVersion() { 

    StringBuffer sb = new StringBuffer();

    sb.append("V0.01")
    sb.append(", native: ibfs32.dll(")
    sb.append(DotNetAdapter.getToken(this.mainVersionBuffer.ToString(), 2).ToString())
    sb.append(") type")
    sb.append(this.portType).ToString())
    sb.append(":")
    sb.append(DotNetAdapter.getToken(this.typeVersionBuffer.ToString(), 0xff).ToString())
    sb.append("
    sb.append(DotNetAdapter.getToken(this.typeVersionBuffer.ToString(), 2).ToString())
    sb.append(")");

    return sb.ToString();
    }[/code]

    That might be closer to some reasonable code :)

  • (cs)

    Ya'know, that just baffles me.  How do you think to do something like that?  I mean you're already using append, so you know it exists.

  • (cs)

    That's the craziest thing I've ever seen. I mean, having "V0.01" and "ibfs32.dll" hardcoded? That's laughable!

  • (unregistered)

    I have seen the light. The dog ate my homework and the pattern confused me [+o(]

  • (cs) in reply to
    :
    I have seen the light. The dog ate my homework and the pattern confused me [+o(]
  • (unregistered) in reply to fcarlier

    String.Format("V0.01, native: ibfs32dll({0})type {1}:{2}", ... ) is probably even a little more reasonable than that, and still runs in linear time.

  • (cs) in reply to

    Have to agree with String.Format route here...makes more sense in with as static format.

  • (cs) in reply to Steve O.
    Steve O.:
    Have to agree with String.Format route here...makes more sense in with as static format.


    Trying again with proper English.   Using the String.Format makes more sense when you have a static format.
  • (cs)

    <font style="BACKGROUND-COLOR: #efefef">string.Format is my best friend in life.</font>

  • (cs) in reply to seizethedave

    That made me want to turn religious. It's people like him that are going to destroy the world someday.

  • (cs)

    I think the reasoning behind this snippet was something like:

    • String concatenation is bad.
    • If you want to concatenate Strings I have to use StringBuffer append method.
    • But if I pass Strings to the append method I'm now concatenating StringBuffers with Strings, that's obviously not the optimal solution.
    • If I wrap all those little Strings into StringBuffers I'll be concatenating StringBuffers with StringBuffers, that way the program will fly.
    • Damn!... append methods doesn't accept StringBuffers directly, I'll have to use ToString on my brand new shiny StringBuffers, not the optimal solution but the best I can do given those stupid BCL classes...

    Sometimes shallow knowledge is worst than total ignorance.

  • (unregistered) in reply to DanielMoth

    From what I understand the compiler would optimize this line 

    sb.Append("some string" + "more" + "and so on");

    to 

    sb.Append("some stringmoreand so on");

    if however it was

    string a ="this";
    string b= " is silly";

    sb.Append(a+b);

    we would have another WTF runner up





  • (cs)

    Umm...

    Uargh...

    Wow?

    Yeah! That's it! WOW! That's ingenious! It takes a freak of nature to figure out such a clever way to use the StringBuilder class and annul StringBuilder's advantages at the same time.

    StringBuilder is nice... I remember in by rookie .NET days, before I knew about string's immutability, I wrote a custom color-coded XML editor control that would parse through and tree format the XML information.

    Some of the documents I used to test it were close to a meg in size, and it would take 5 to 10 minutes to parse and format using string. I changed to StringBuilder, and VOILA, it took less than three seconds!

  • (unregistered)

    This would make sense if... if... hmmmm ah forget it, even I can't justify this one [|-)]

  • (cs) in reply to Guayo
    Guayo:
    Damn!... append methods doesn't accept StringBuffers directly


    I don't do J# - I'm a C# or VB.NET man myself, but StringBuilder.Append does accept another StringBuilder.

    I think this is a case of a little knowledge being very dangerous. The thought process probably went something like:
    "Hmm. I need to concatenate some strings, but - being a top flight developer - I know that string concatenation is a Bad Thing. I know! I've heard that StringBuilders are brilliant! And if I use lots of them, my code will be even better!"

    Guayo:
    Sometimes shallow knowledge is worst than total ignorance.


    Sums it up perfectly. WTF!!!!!
  • (cs) in reply to Rob
    Rob:
    [image] Guayo wrote:
    Damn!... append methods doesn't accept StringBuffers directly


    I don't do J# - I'm a C# or VB.NET man myself, but StringBuilder.Append does accept another StringBuilder.
    [...]


    AFAIK it does not (at least in 1.1) accept StringBuilders but does accept objects (so it's not a compilation error to call it with another StringBuilder), I'm pretty sure that the overloaded implementation will call the ToString method on the object passed. It  could, however, test inside the append method to see if the object passed is an instance of the StringBuilder class… ?

    Java StringBuffer does accept StringBuffers to its append method.

    As this is J# I supose StringBuffer it's just an alias to StringBuilder.
  • (cs)

    Could it be the result of a macro to remove string concatenation?

  • (unregistered)

    Looks like someone has an obsession with functional languages.  Perhaps what we have here is a Lisp afficiondo.

  • (cs) in reply to Guayo
    Guayo:
    [image] Rob wrote:
    [image] Guayo wrote:
    Damn!... append methods doesn't accept StringBuffers directly


    I don't do J# - I'm a C# or VB.NET man myself, but StringBuilder.Append does accept another StringBuilder.
    [...]


    AFAIK it does not (at least in 1.1) accept StringBuilders but does accept objects (so it's not a compilation error to call it with another StringBuilder), I'm pretty sure that the overloaded implementation will call the ToString method on the object passed. It  could, however, test inside the append method to see if the object passed is an instance of the StringBuilder class… ?

    Java StringBuffer does accept StringBuffers to its append method.

    As this is J# I supose StringBuffer it's just an alias to StringBuilder.
  • (cs)

    Doesn't this create a huge memory leak? Of course, it is just getClassVersion... But what if this method is used everywhere over the program, even in loops?

  • (cs) in reply to aapopfriets

    Of course with a method like this where it will not be called very often, readability is more important than effificency, so string concatenation would have been equally acceptable (as the proper StringBuilder implementation, not this stupid WTF implementation).

    Somtimes I think people here get a bit obsessed with what is the most efficient way, and forget that more readable is often better

  • (unregistered) in reply to Modan
    Modan:
    Somtimes I think people here get a bit obsessed with what is the most efficient way, and forget that more readable is often better


    Didn't you mean 'too many people, but especially novices' and not 'people here'? [:)] As Knuth said, "premature optimisation is the root of all evil". Too often I see people concerned with performance who (pick the combination as required): 1. Haven't finished implementing their solution, 2. Neglected design (be it code first, simply poor, etc.), 3. Haven't profiled (WTF!), 4. Haven't considered the tradeoffs or even whether the solution will do as-is, 5. Don't know enough to make an informed decision. I'm sure there are others - those are just off the top of my head.

    Amateur / hobbyist game developers usually provide great examples of this, sometimes with some truly mad ideas. :)
  • (unregistered)

    What's wrong with a good and clean

    sprintf(buffer, "%s %s %u stuff %s %s more stuff", var1, var2, number1, ..etc);

  • (unregistered) in reply to aapopfriets
    aapopfriets:
    Doesn't this create a huge memory leak? Of course, it is just getClassVersion... But what if this method is used everywhere over the program, even in loops?


    Wheres the leak sir?   
  • (cs)

    I definitely appreciate the wonderful irony of this WTF: 

    To save memory, he used a StringBuilder class,  but then used it in such a way that is uses even MORE memory (and is much more convoluted) than it would have been if he left it as is !

    I can only imagine what the rest of the code looked like.

  • (cs)

    The hilarity of this web site has caused me to create an account. Makes me wonder how I did not hear of this place before now!

    This StringBuilder is a great WTF!

  • (cs)

    Can't you just do:

    public override string getClassVersion() {
    return new StringBuffer().append("V0.01").append(", native: ibfs32.dll(").append(DotNetAdapter.getToken(this.mainVersionBuffer.ToString(), 2).ToString()).append(") type").append(this.portType).ToString()).append(":").append(DotNetAdapter.getToken(this.typeVersionBuffer.ToString(), 0xff).ToString()).append(".append(DotNetAdapter.getToken(this.typeVersionBuffer.ToString(), 2).ToString()).append(")");
    }

    ???
    I'm just wondering. I know nothing about J#...

  • (cs) in reply to aapopfriets
    aapopfriets:
    Doesn't this create a huge memory leak? Of course, it is just getClassVersion... But what if this method is used everywhere over the program, even in loops?


    Even if this method were used in a huge loop it wouldn't make the app crash as there isn't a permanent reference for all those temporary objects. Of course using all those objects is a waste but that alone does not make a memory leak.
  • (cs) in reply to Katja
    Katja:
    Can't you just do:
    public override string getClassVersion() {
    return new StringBuffer().append("V0.01").append(", native: ibfs32.dll(").append(DotNetAdapter.getToken(this.mainVersionBuffer.ToString(), 2).ToString()).append(") [type").append(this.portType).ToString()).append(":").append(DotNetAdapter.getToken(this.typeVersionBuffer.ToString(), 0xff).ToString()).append("](").append(DotNetAdapter.getToken(this.typeVersionBuffer.ToString(), 2).ToString()).append(")");
    }
    ???
    I'm just wondering. I know nothing about J#...


    Yes, you can do that but it does look ugly. [;)]
    I'm partially kidding, of course it's a lot better than the posted WTF and you will using the StringBuilder/StringBuffer efficiently, It's just that I think putting all those appends together hurts readability.
  • (unregistered) in reply to
    :
    Am I missing something? [*-)] What's wrong with this?
  • (unregistered) in reply to DanielMoth
    DanielMoth:

    In the past I have smiled when coming across something like:

        StringBuilder sb = new StringBuilder();
        sb.Append("some string" + "more" + "and so on");

    but creating a brand new stringbuilder each time is ingenious. Well done whoever!



    Concating string literals is not a problem - It's a problem when you use string vaariables,
  • (cs) in reply to aapopfriets

    Ahum. No, it doesn't create a memory leak at all - .NET uses garbage collection so any StringBuilder object will be cleaned up some time after all references to it are lost. In casu: after the method has finished executing.

    Garbage collection is a wonderfull thing!

  • (cs)

    Just use the + operator: it'll effect a call to String.Concat unless the concatenation can be performed at compile time.

    e.g.:

    [code language="c#"]
    string a = "one" + "two" + "three"; // concatenated at compile-time
    string b = 1.ToString() + 2.ToString() + 3.ToString(); // effects a call to String.Concat
    [/code]

  • (cs) in reply to Tom

    Urgh... that didn't come out like in the preview.

  • (cs) in reply to fcarlier
    fcarlier:
    Ahum. No, it doesn't create a memory leak at all - .NET uses garbage collection so any StringBuilder object will be cleaned up some time after all references to it are lost. In casu: after the method has finished executing.

    Garbage collection is a wonderfull thing!


    As a matter of fact, wouldn't you agree that it's not even possible (in managed code) to create a memory leak (by definition)?

    I can't think of a case where you could.

    Kelly
  • (cs) in reply to kellyleahy
    kellyleahy:

    [...]
    As a matter of fact, wouldn't you agree that it's not even possible (in managed code) to create a memory leak (by definition)?
    I can't think of a case where you could.
    Kelly


    That depends of your definition of memory leak.
    For me a memory leak is a condition made by a program error which causes unused memory not to be returned to the execution environment thus making memory consumption permanently increase and sometimes causing the program to crash.
    You definitely can do this in .Net or any other managed environment just by not releasing references of temporal objects (this is not the case of the snipped posted).
    I agree that the CLR (or the JVM for that matter) can make your app crash gracefully but if happens because it gets out of memory and the cause of that was a lot of unused objects whose reference was kept, that would be a memory leak in my book.
  • (cs) in reply to Guayo
    Guayo:
    [image] kellyleahy wrote:

    [...]
    As a matter of fact, wouldn't you agree that it's not even possible (in managed code) to create a memory leak (by definition)?
    I can't think of a case where you could.
    Kelly


    That depends of your definition of memory leak.
    For me a memory leak is a condition made by a program error which causes unused memory not to be returned to the execution environment thus making memory consumption permanently increase and sometimes causing the program to crash.
    You definitely can do this in .Net or any other managed environment just by not releasing references of temporal objects (this is not the case of the snipped posted).
    I agree that the CLR (or the JVM for that matter) can make your app crash gracefully but if happens because it gets out of memory and the cause of that was a lot of unused objects whose reference was kept, that would be a memory leak in my book.


    I guess we differ in opinion on that one.  I think if the programmer kept a bunch of "valid" references that they didn't need, that's a poor implementation, but not necessarily a memory leak.  To me a memory leak is when the programmer thinks they aren't keeping a reference to something and are freeing it, but in reality aren't doing either correctly, that it is a memory leak.  To me, a memory leak can thus only exist on systems where memory is not managed (or where circular references are possible - as in COM).

    Kelly
  • (cs) in reply to kellyleahy
    kellyleahy:


    As a matter of fact, wouldn't you agree that it's not even possible (in managed code) to create a memory leak (by definition)?

    I can't think of a case where you could.

    Kelly


    Well, obviously, you could when you are using interop with unmanaged code. Think of file streams, database connections, bitmaps in memory and so on.

    Memory leaks could be more subtle as well. Imagine this:

    object A (the parent object), which holds a reference to object B (a child object)
    object B (the child object), which in return holds a reference to object A (the parent object).

    If things like that get complicated, you're still running into a memory leak - even in the managed world.
    This, of course, only holds true while your program is running. When it shut down all managed memory will be freed again. By definition.
  • (cs) in reply to fcarlier
    fcarlier:

    Well, obviously, you could when you are using interop with unmanaged code. Think of file streams, database connections, bitmaps in memory and so on.


    thats why I said "managed".  I meant "managed all the way - not calling unmanaged code".

    fcarlier:

    object A (the parent object), which holds a reference to object B (a child object)
    object B (the child object), which in return holds a reference to object A (the parent object).

    If things like that get complicated, you're still running into a memory leak - even in the managed world.
    This, of course, only holds true while your program is running. When it shut down all managed memory will be freed again. By definition.


    at the least, this isn't true in Java.  I suspect it isn't true for .NET languages as well.  In Java, the GC works based on "reachable references" which are references that are live to the program.  If you have object A refers to object B and object B refers to object A, but nothing that the code has a reference to refers to either A or B, then the A and B references are "unreachable" and thus can (and will) be GCed.
  • (cs) in reply to kellyleahy

    kellyleahy:

    [...]
    I guess we differ in opinion on that one.  I think if the programmer kept a bunch of "valid" references that they didn't need, that's a poor implementation, but not necessarily a memory leak.  To me a memory leak is when the programmer thinks they aren't keeping a reference to something and are freeing it, but in reality aren't doing either correctly, that it is a memory leak.  To me, a memory leak can thus only exist on systems where memory is not managed (or where circular references are possible - as in COM).


    So by what you say we could conclude that an unneeded active reference is valid just because the garbage collector can't reclaim that memory? I would think that a reference is valid because the resourse is still needed? Why is that fundamental difference between forgetting to free a chunk of unused memory of an unmanaged app and forgetting to release an unused reference of a managed app? The memory is still wasted, if we keep doing that enough the app will crash.

    Managed app's do suffer a lot less from this ill but they are not immune, it's just that they have a good health care stuff behind (the GC).

    The fact is that no matter if you don't want to call it memory leaks, .Net apps still needs to be memory profiled and that it's not so uncommon that a piece of code makes the app run out of memory because unwanted references are being kept. Sometimes the culprit it's just a static variable or an event handler.

    And what about unmanaged recourses. Almost all managed apps interact with unmanaged resources, resources which the garbage collector is not aware of the cost. Resources that can tear down a managed app.

    And finally what about the .Net framework or Java bugs?

  • (unregistered)

    Anyone had the bright idea that this code is probably converted with some sort of automatic generator from VB or such?

Leave a comment on “The not-so-efficient StringBuilder”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article