• (cs)
        while (size-- > 0) { 
          BehaviorEngineConnection conn = connections.get(getNextIndex()); 
          if (conn.isConnected()) {
             return conn; 
          } 
          LOG.error("No connected connection available!"); 
          return null; 
        } 
    

    So for every connection that is not connected it logs a misleading error message, but keeps on trying size times, with a high probability of successfully getting a connection on one of the following tries, polluting the logs with much errdo about nothing?

    Nice!

  • (cs) in reply to no laughing matter
    no laughing matter:
        while (size-- > 0) { 
          BehaviorEngineConnection conn = connections.get(getNextIndex()); 
          if (conn.isConnected()) {
             return conn; 
          } 
          LOG.error("No connected connection available!"); 
          return null; 
        } 
    
    So for every connection that is not connected it logs a misleading error message, but keeps on trying size times, with a high probability of successfully getting a connection on one of the following tries, polluting the logs with much errdo about nothing?

    Nice!

    Not quite. It returns.

  • (cs) in reply to Memleak
    Memleak:
    Not quite. It returns.
    Ouch! Even more WTF: A while-loop that does not loop at all!
  • faoileag (unregistered)

    It doesn't matter that getNextIndex() is public - getNextIndex() implements the round robin and provides the starting index for looking for a connected connection.

    If it is called from outside, all that happens is that the search for a connected connection starts at a different index, but the connection pool is still searched in full...

    ...or rather, it would be if the developer hadn't (accidentally?) placed his "return null" inside the while loop instead of after it.

  • faoileag (unregistered) in reply to no laughing matter
    no laughing matter:
    Memleak:
    Not quite. It returns.
    Ouch! Even more WTF: A while-loop that does not loop at all!
    Like the old saying from agile development: "It's not iteration if you only do it once."
  • (cs) in reply to faoileag
    faoileag:
    Like the old saying from agile development: "It's not iteration if you only do it once."
    And it's not reiteration unless you do it at least three times.
  • (cs)

    TRWTF: using synchronizedList but not synchronizing access to "useNext" you might as well not have any synchronization at all then

  • lol (unregistered)

    so another WTF where the WTRF is actually the article itself, reporting a reasonable piece of code, where the reporter is quick to point out non-flaws but not understanding the real error in the codebase at all.

    I blame the kids who try to code nowadays, too quick to judge, not mature enough to take time to understand what they're doing.

  • Meep (unregistered) in reply to ratchet freak
    ratchet freak:
    TRWTF: using synchronizedList but not synchronizing access to "useNext" you might as well not have any synchronization at all then

    Either the synchronization of the synchronizedList would do nothing, or you'd wind up with deadlock. You can't compose locks in any useful manner, deadlock or livelock always screw you.

    TRWTF is synchronizedList, which just exists because Sun needed something to replace Stack and Vector and HashTable. The only way a synchronized container is useful is if everything you do can be expressed atomically.

    For instance, with an ArrayList you can call .remove(0) and get an object, and then do something with it. For a connection pool, it's pretty reasonable to remove a connection in use, and add it back when you're done. (At which point, though, it looks like you probably want a Queue.)

    In this case, though, it would make more sense to just have the set of connections be regular array unless you know for certain thousands of connections would work.

  • noname (unregistered)

    Got this error myself once. An increment is not an atomic operation so being scheduled away after reading but before writing can lead to returning the same index twice on different threads.

  • faoileag (unregistered) in reply to lol
    lol:
    I blame the kids who try to code nowadays, too quick to judge, not mature enough to take time to understand what they're doing.
    Have some sympathy, it's not as easy as it was in the olden times...

    When I got my first home computer, what you had to learn was BASIC. You read a (slim) manual on the language, hacked a few games into the machine from a book or a magazine and you were basically set up. Perhaps you'd learn a bit of machine language to speed things up, but the Z80 instruction set was also pretty concise.

    10 years later and the number of pages in programming books had more than quadrupled and the books would still cover only the basics. Multithreading? Not even mentioned!

    So these days I wouldn't blame young coders too much for not knowing what they are doing. Extended puppy licence if you want.

    Yes, it would be great if the coder when perhaps copying and pasting some construct from some older module would look up features like synchronizedCollection in the docs to see what they are doing. But for that you need time and that means you need a company that gives you the time.

    But a lot of companies don't give you the time because they don't see the software their developers create as an investment. The software is buggy? Doesn't matter, get some cheap code monkeys to make it work and move on.

    And let's be honest: would the situation be different we wouldn't have a website like TDWTF.

  • faoileag (unregistered) in reply to noname
    noname:
    Got this error myself once. An increment is not an atomic operation so being scheduled away after reading but before writing can lead to returning the same index twice on different threads.
    Actually, with enough threads calling getNextIndex() and a small connection pool, a very slim chance exists that thread X always gets the same index while looping the loop and if that connection is not connected will never reveice a connection although perhaps two usable ones exist in the pool.

    This piece of code would make a nice example of a connection pool that is not thread safe...

  • (cs) in reply to faoileag
    faoileag:
    ... agile development: ...
    But it did work in the Unit tests! (Which only tested with a single thread).
  • faoileag (unregistered) in reply to no laughing matter
    no laughing matter:
    faoileag:
    ... agile development: ...
    But it did work in the Unit tests! (Which only tested with a single thread).
    Actually, I think you can't really unit test thread safety. Unit tests are too formal; they test that with input x the expected (for x) result y is returned.

    For multithreading I would set up a stress test and let it run over night. With a mocked BehaviorEngineConnection you can easily track distribution of returned connections, you can log any exceptions with a generic catch and when logging constructor calls in the RoundRobinConnectionContainer's constructor you can also catch unexpected system restarts.

  • faoileag (unregistered) in reply to no laughing matter
    no laughing matter:
    faoileag:
    ... agile development: ...
    But it did work in the Unit tests! (Which only tested with a single thread).
    Got one more:
    assert(isLeapYear(2004));
    boolean isLeapYear(int year) {
      return true;
    }
    "But the unit tests all passed!!!"
  • Sociopath (unregistered) in reply to Meep
    Meep:
    For instance, with an ArrayList you can call .remove(0) and get an object, and then do something with it. For a connection pool, it's pretty reasonable to remove a connection in use, and add it back when you're done. (At which point, though, it looks like you probably want a Queue.)
    One lazy and/or ignorant client, and your whole connection pool is drained. Brillant!
  • Inc (unregistered) in reply to no laughing matter

    Even worse, it looks like if this.connections.size() = 0, then it doesn't return any value.

    Really the problem is one of the curly brackets needs to be moved up two lines.

  • mar77i (unregistered)

    Whee are we referencing which connection is the right one? This can't seriously be a fifo if it's asynchronous...

  • Does That Work? (unregistered)

    The getNextIndex() will never return 0. It will return .Size(), then next call resets the counter.

    For those who see no problems, is connections using a 1-base array count when everything else in Java is 0-base? (It could be, I'm not using them and not taking the time to research it.)

  • (cs) in reply to Does That Work?
    Does That Work?:
    The getNextIndex() will never return 0. It will return .Size(), then next call resets the counter.

    For those who see no problems, is connections using a 1-base array count when everything else in Java is 0-base? (It could be, I'm not using them and not taking the time to research it.)

    Welcome to the wonders WTFs of Cs post-XXX-operators (which Java inherited from C):

    The following line:

    return( this.useNext++ );
    

    is a post-increment-operator in operation: It will return the old value of this.useNext, then increment it and store it back in the variable.

    So when the size-test has set useNext to 0, the method will return 0 and the following call will see a 1.

    So this is actually working, but it's confusing as hell.

    And indeed everything in Java is zero-based.

    Except JDBC column-indexes, which are a WTF on its own.

  • tldr (unregistered)

    Will someone read this out load and send my the sound file?

  • Zynaps (unregistered) in reply to Inc
    Inc:
    Even worse, it looks like if this.connections.size() = 0, then it doesn't return any value.

    Really the problem is one of the curly brackets needs to be moved up two lines.

    Finally someone got it. Not really a WTF, just another simple bug, probably due to bad indentation on the original code.

    TRWTF, as usual, is people on this site should know how to analyze code, but don't. This just reflects the general level of developer competence out there, where only 1 out of 10 developers know how to read other people's code.

  • (cs) in reply to Zynaps
    Zynaps:
    Finally someone got it. Not really a WTF, just another simple bug, probably due to bad indentation on the original code.

    TRWTF, as usual, is people on this site should know how to analyze code, but don't. This just reflects the general level of developer competence out there, where only 1 out of 10 developers know how to read other people's code.

    "That incompetent compiler! He doesn't know how to read my code!"
  • (cs)

    TRWTF is still that Java thinks that size should be a function, not a property.

  • (cs) in reply to Zynaps
    Zynaps:
    Inc:
    Even worse, it looks like if this.connections.size() = 0, then it doesn't return any value.

    Really the problem is one of the curly brackets needs to be moved up two lines.

    Finally someone got it. Not really a WTF, just another simple bug, probably due to bad indentation on the original code.

    TRWTF, as usual, is people on this site should know how to analyze code, but don't. This just reflects the general level of developer competence out there, where only 1 out of 10 developers know how to read other people's code.

    It's easier just to /* do this */ and create code that you understand, even if it doesn't do the same thing.

  • ... (unregistered) in reply to tldr
    tldr:
    Will someone read this out load and send my the sound file?
    Better yet, can someone do that and then send me the transcript of it?
  • D B (unregistered) in reply to chubertdev
    chubertdev:
    TRWTF is still that Java thinks that size should be a function, not a property.

    TWTF is having a difference.

  • (cs) in reply to D B
    D B:
    chubertdev:
    TRWTF is still that Java thinks that size should be a function, not a property.

    TWTF is having a difference.

    some joke about you not "getting it"

  • D B (unregistered) in reply to chubertdev
    chubertdev:
    D B:
    chubertdev:
    TRWTF is still that Java thinks that size should be a function, not a property.

    TWTF is having a difference.

    some joke about you not "getting it"

    Turned insult?

  • (cs)

    No, just accessor-related humor.

    Why don't you believe that functions and properties should be separate entities?

  • Jizz (unregistered)

    Go lay an egg you commie bastards

  • D B (unregistered) in reply to chubertdev
    chubertdev:
    No, just accessor-related humor.

    Why don't you believe that functions and properties should be separate entities?

    I don't code Java so you would be educating me on their advantages, but in the end it doesn't really matter because a property is an implementation of a function.

    Why complicate things by treating one function different from another?

  • (cs) in reply to D B
    D B:
    chubertdev:
    No, just accessor-related humor.

    Why don't you believe that functions and properties should be separate entities?

    I don't code Java so you would be educating me on their advantages, but in the end it doesn't really matter because a property is an implementation of a function.

    Why complicate things by treating one function different from another?

    What do you code in?

    I can see the JavaScript now:

    function GetValueOfVariable()
    { ... }
    
    function SetValueOfVariable()
    { ... }
    
  • (cs)

    I also have the name of today's article stuck in my head to the tune of, "Sir Robin Ran Away."

  • D B (unregistered) in reply to chubertdev
    chubertdev:
    D B:
    chubertdev:
    No, just accessor-related humor.

    Why don't you believe that functions and properties should be separate entities?

    I don't code Java so you would be educating me on their advantages, but in the end it doesn't really matter because a property is an implementation of a function.

    Why complicate things by treating one function different from another?

    What do you code in?

    I can see the JavaScript now:

    function GetValueOfVariable()
    { ... }
    
    function SetValueOfVariable()
    { ... }
    

    More like:

     Function() Returns Value
    
     Function(NewValue) Sets Value
    

    ...

    All code. Depending on your level of separation, whether you use a single function or two... internally they're still functions. Properties are an implementation, a perspective.

    CAPTCHA: incassum

  • Mason Wheeler (unregistered) in reply to D B
    D B:
    chubertdev:
    No, just accessor-related humor.

    Why don't you believe that functions and properties should be separate entities?

    I don't code Java so you would be educating me on their advantages, but in the end it doesn't really matter because a property is an implementation of a function.

    Why complicate things by treating one function different from another?

    Because properties are not "an implementation of a function;" a function is one implementation of a property. It's also possible for a property accessor to directly deal with a backing field. It's possible for a property's read accessor to pass directly through to a backing field, and for the write accessor to call a method. (ie: I just changed the size of a form; set the new value and then repaint.)

    And here's the beautiful part: it's possible to take a property that's backed by a field, and replace its accessor with a function, or vice versa, and all code that references it continues to function without even noticing the difference.

  • (cs)

    I'll post this again: Luddites!

    Probably doesn't even use stored procedures.

  • LoztInSpace (unregistered)

    The real WTF is application developers having to write connection pools instead of something useful to support their business.

  • Norman Diamond (unregistered)

    It doesn't matter what's right. What matters is who has the most connections.

  • (cs) in reply to LoztInSpace
    LoztInSpace:
    The real WTF is application developers having to write connection pools instead of something useful to support their business.

    What, you don't like doing your own garbage collection?

  • Captain Oblivious (unregistered) in reply to LoztInSpace
    LoztInSpace:
    The real WTF is application developers having to write connection pools instead of something useful to support their business.

    Myeh.

    On the one hand, you're entirely right.

    On the other, a pool is just a fancy data structure that does a thing. That's exactly what application developers should be "writing" -- by adding semantics to built-in data structures.

    On the third hand, some highly productive languages are highly productive because they make it trivial to write complex data structures.

  • (cs) in reply to Captain Oblivious
    Captain Oblivious:
    LoztInSpace:
    The real WTF is application developers having to write connection pools instead of something useful to support their business.

    Myeh.

    On the one hand, you're entirely right.

    On the other, a pool is just a fancy data structure that does a thing. That's exactly what application developers should be "writing" -- by adding semantics to built-in data structures.

    On the third hand, some highly productive languages are highly productive because they make it trivial to write complex data structures.

    A connection pool isn't exactly something that the average developer should have to author. They should understand it, no doubt. But it's not a problem that's specific to their program, in fact, it's in the majority of programs, so it's something that should already have a solution.

  • Captain Oblivious (unregistered) in reply to chubertdev
    chubertdev:
    Captain Oblivious:
    LoztInSpace:
    The real WTF is application developers having to write connection pools instead of something useful to support their business.

    Myeh.

    On the one hand, you're entirely right.

    On the other, a pool is just a fancy data structure that does a thing. That's exactly what application developers should be "writing" -- by adding semantics to built-in data structures.

    On the third hand, some highly productive languages are highly productive because they make it trivial to write complex data structures.

    A connection pool isn't exactly something that the average developer should have to author. They should understand it, no doubt. But it's not a problem that's specific to their program, in fact, it's in the majority of programs, so it's something that should already have a solution.

    Yeah, probably not. But then again, what would it take? A collection type that acts as a functor, a morphism in the Kliesli category to make the connections, a morphism in the Eilenberg-Moore category to close them, and an adjunction to check the connection out and return it.

    Trivial.

  • (cs) in reply to Captain Oblivious
    LoztInSpace:
    The real WTF is application developers having to write connection pools instead of something useful to support their business.

    Well, somebody has to write them!

    Captain Oblivious:
    On the one hand, you're entirely right.

    On the other, a pool is just a fancy data structure that does a thing. That's exactly what application developers should be "writing" -- by adding semantics to built-in data structures.

    On the third hand, some highly productive languages are highly productive because they make it trivial to write complex data structures.

    Man, I sure wish I had a third hand.

  • D B (unregistered) in reply to Mason Wheeler
    Mason Wheeler:
    D B:
    chubertdev:
    No, just accessor-related humor.

    Why don't you believe that functions and properties should be separate entities?

    I don't code Java so you would be educating me on their advantages, but in the end it doesn't really matter because a property is an implementation of a function.

    Why complicate things by treating one function different from another?

    Because properties are not "an implementation of a function;" a function is one implementation of a property. It's also possible for a property accessor to directly deal with a backing field. It's possible for a property's read accessor to pass directly through to a backing field, and for the write accessor to call a method. (ie: I just changed the size of a form; set the new value and then repaint.)

    And here's the beautiful part: it's possible to take a property that's backed by a field, and replace its accessor with a function, or vice versa, and all code that references it continues to function without even noticing the difference.

    Fuji's and Gala's.

    http://stackoverflow.com/questions/3996778/what-is-the-difference-between-readonly-property-and-function-in-net

    --

    But yes, I should have taken architecture into consideration. I was coming from the perspective where imposing syntax restrictions interferes with properties and functions as overloaded properties.

    So for example (and this probably isn't true), if XXX.ToString worked for a property, but didn't work for a function. It shows that down in the code they're really both functions, but because the implementation treats them differently it then ends up causing more headaches than it solves.

  • (cs) in reply to D B
    D B:
    So for example (and this probably isn't true), if XXX.ToString worked for a property, but didn't work for a function. It shows that down in the code they're really both functions, but because the implementation treats them differently it then ends up causing more headaches than it solves.
    Speaking of which, why is XXX.ToString() a function/method in the first place? Shouldn't it be just the XXX.string property?
  • faoileag (unregistered) in reply to Zynaps
    Zynaps:
    Inc:
    Even worse, it looks like if this.connections.size() = 0, then it doesn't return any value.
    Finally someone got it. Not really a WTF, just another simple bug, probably due to bad indentation on the original code.
    Actually, that makes me wonder if that code ever reached production stage or if some code is missing for obfuscation. As ar as I know the java compiler, it should croak on a function that does not return a value.
  • Maurizio (unregistered) in reply to Mason Wheeler
    Mason Wheeler:

    And here's the beautiful part: it's possible to take a property that's backed by a field, and replace its accessor with a function, or vice versa, and all code that references it continues to function without even noticing the difference.

    And it is possible to write a function accessor that use a backing field, and when you change it a something different, it continues to function without even noticing the difference.

    TRWTF is accessing instance variables by names instead that with a function (or a property). Flavors rules.

  • your name (unregistered) in reply to chubertdev
    chubertdev:
    LoztInSpace:
    The real WTF is application developers having to write connection pools instead of something useful to support their business.

    What, you don't like doing your own garbage collection?

    Around here the Programmers' Mafia controls all the garbage collection...

  • (cs) in reply to faoileag
    faoileag:
    Actually, that makes me wonder if that code ever reached production stage or if some code is missing for obfuscation. As ar as I know the java compiler, it should croak on a function that does not return a value.

    That's a good point, this would not compile because it is not guaranteed to return.

Leave a comment on “This Round Robin Laid an Egg”

Log In or post as a guest

Replying to comment #430459:

« Return to Article