• (cs)

    Not the frist time I've seen code that bad.

  • Ama (unregistered)

    The code was working, they had other priorities to code first. Where's the WTF?

  • grabah (unregistered)

    if it works it works.

  • Fingie (unregistered) in reply to Ama
    Ama:
    The code was working, they had other priorities to code first. Where's the WTF?

    Maybe this is a case of sloppy WTF.

  • (cs)

    The real WTF is how often people look at unmaintainable code and say, "Meh, it works, right?"

  • (cs) in reply to Ama
    Ama:
    The code was working, they had other priorities to code first. Where's the WTF?

    Probably the manner of the reply?

    It's one thing to say items x,y and z are ahead of refactoring in the queue of things-to-do but chiding someone for even thinking it needed to be addressed smells of a bad development culture.

  • furiant (unregistered)

    Seemingly satisfied with this story, I nod in apparent satisfaction as I jot down this quick comment.

  • letatio (unregistered)

    What the other developers were saying had some truth to it. They failed in expressing what they meant. Any skilled developer with a few years experience knows that as a rule of thumb, don't fix it if it isn't broken. Re-factoring anything is a sure way to introduce new bugs.

    However, there are sometimes better ways to remedy this. Did the developer propose that they move towards TDD?

  • (cs)

    It looks like Mark Bowytz has created his own (more manly) version of Cornify. Click on "emergency bug"!

  • Meep (unregistered) in reply to Remy Porter
    Remy Porter:
    The real WTF is how often people look at unmaintainable code and say, "Meh, it works, right?"

    What you do is realize:

    a. if you break it, you're the fuckup

    b. other people will keep writing more shit code.

    So you conclude that you'll just try to keep your code clean.

    Until people shit all over that, too.

  • Meep (unregistered)
    The hiring manager cocked an eyebrow and replied, "What about a scenario where it's 2 in the morning and you have to apply an emergency bug fix ASAP. Sometimes, to get things moving, maybe you have to cut an occasional corner?"

    If it's an emergency, then it really needs to be done right or you'll wind up having to patch the patch. It's the "do no harm" principle: you never knowingly commit deficient, uncommented or untested code.

  • (cs)

    The indent is way too big. I could never work there.

  • some random cat (unregistered) in reply to Ama
    Ama:
    The code was working, they had other priorities to code first. Where's the WTF?

    Relax your high standards about WTFs, man

  • (cs)
    stmtInsertToTarget.setInt(1, rsSource.getInt(1));

    TRWTF is limiting your application to Integer.MAX_VALUE records.

  • (cs) in reply to Ama
    Ama:
    The code was working, they had other priorities to code first. Where's the WTF?

    I came here to say this.

    If you came in a meeting and said "Hey, let's take this ugly code that works and spend 4 or 5 hours rewriting it so that it's not ugly" I'd respond the same way. Yes, it needs to be done, but 4-5 hours worth of code changes will have to be followed with 9 or 10 hours of testing. And if there is even ONE bug... one little tiny bug in the refactored code, there's going to be bitching from some other department about why we "fixed" something that wasn't broken.

    So, yes, put in a ticket to get it fixed, but that ticket will be a much, much lower priority than all the other things that need to get done.

    Now, on the other hand, if there is even ONE bug with big ugly code, you now have an excuse to make some changes, and in the midst of those changes "sneakily" clean up the surrounding ugly code in the name of bug fixing :)

  • ColdHeart (unregistered)

    I think this and many other examples point out how important it is to ask an interviewer about the practices of the company.

    It's one thing to say "We have other priorities atm, so that can wait" and "if it aint broke, don't fix it". With something like this, fixing it now can help adding to it later.

  • Steven Seagal's ponytail (unregistered) in reply to Meep
    Meep:
    What you do is realize:

    a. if you break it, you're the fuckup

    b. other people will keep writing more shit code.

    So you conclude that you'll just try to keep your code clean.

    Until people shit all over that, too.

    Having worked in a "legacy" environment, this was exactly the mindset that I fell into. However, the old dinosaurs would jump all over my code for using newer features of the language because they didn't understand it. I think I still have scars from some of those battles.

  • Foo Bar (unregistered)

    Refactoring that code (and its numerous siblings) with something more versatile and generic would almost certainly result in a net decrease in LoC, therefore bad.

    Unroll ALL the loops!

  • peter (unregistered) in reply to Ama
    Ama:
    The code was working, they had other priorities to code first. Where's the WTF?

    The problem with this approach is that, a handful of years or so down the line, the codebase will be an impenetrable mess, and simple maintenance will be a task much more daunting (and time-consuming) than refactoring would have been now. Also, it will suck the will to live out of the developers that are responsible for maintenance.

    On top of that, the harder it is to figure out what unreadable code, written by people who are no longer around, does, the more insurmountable the task of refactoring it will be.

    Don't ask me how I know this.

    captcha: abigo: a big null?

  • MuTaTeD (unregistered)

    I inherited the similar db access code when I joined my current employer as team lead. It took me almost a year to convince the client that this section needed to be refactored and in the mean time I left this section to the developer who was already working when I joined.

    Finally after one year of convincing the client and with the application crashing on database crashing when the application went multi-threaded, they finally agreed. I spent 2-3 days on the actual refactoring including the planning phase and a couple of days more for testing out the changes.

    Now I am happy to say that the database section is quite solid and can handle simultaneous operations without much complaints.

  • Mike (unregistered)

    So it seems as if the interviewer knows of the problem and is hiring people who may be able to fix it, yet has no ability to change the culture to enable that to happen.

    Management failure, sack the beardies.

  • AGray (unregistered) in reply to ubersoldat
    ubersoldat:
    stmtInsertToTarget.setInt(1, rsSource.getInt(1));

    TRWTF is limiting your application to Integer.MAX_VALUE records.

    Could be Decimal.MaxValue instead. Still gives you 999,999,999,999,999,999,999,999,999,999 records, provided it's never negative. (If it's negative, then you've got a measly 999,999,999,999,999,999,999,999,999 max records to work with. Rats.)

    CAPTCHA: duis. Dissidia: Duisdecim...

  • AGray (unregistered) in reply to AGray
    AGray:
    Could be Decimal.MaxValue instead. Still gives you 999,999,999,999,999,999,999,999,999,999 records, provided it's never negative. (If it's negative, then you've got a measly 999,999,999,999,999,999,999,999,999 max records to work with. Rats.)

    ...EDIT: I was wrong. Decimal ranges from: -79,228,162,514,264,337,593,543,950,335 to 79,228,162,514,264,337,593,543,950,335. Sorry about that.

  • Dave (unregistered)

    It was my first real developer job, which means that instead of writing code "under the radar" to help me do the sys admin stuff, I was actually hired to write code.

    During the interview I asked "Do you have written coding standards?" "Oh yes of course." they said. I neglected to ask "Do you follow them?"

    I found out that all my co-developers had no training in development, or software lifecycles, or anything to do with I.T. at all. No, instead they were all proud of their Manufacturing certifications, which I, to their disdain, did not have.

    The code was a mass of unreadability due in part to the habit of cutting one or a few lines of code from this module and pasting them into that module without paying any attention whatsoever to the indent level. Fortunately the editor had a built-in "reformat" command which no one had ever been curious enough to discover. I gradually tidied things up as I touched one program after another.

    When it came time for the next release, a shitstorm arose. I learned that our "customer service" people would take a tape (yes) of the software to each customer and install it. However, they'd all made custom tweaks at each location which of course would be overwritten by the new version.

    The standard practice for this dilemma, unique in the software industry and thus never before solved elsewhere, was to run a "diff" on the customer's version vs. the new release, and manually decide which lines to keep or replace. But with the reformatted code, every line was different! The "customer service" people wanted to kill me.

    OK in retrospect I decided to make a change to production code without knowing all the ramifications. Live and learn.

    But I showed them "diff -w".

    Years in business, dozens of "developers" and "customer service" people, and nobody had ever bothered to RTFmanpage!

  • (cs)

    The code looks WTF'y but I challenge the posters here to write the fix.

    You'd ideally want a schema object, that will run through the various possible types.

    Also please show me how this schema adds any improvement, given that you are going to populate it anyway somewhere. Hard-coded?

    By the way, it is a huge issue I have with this site that you cannot see the article whilst you are writing your comment (unless you open another browser window, of course).

  • (cs)

    This story reminds me of two things:

    1. In July I was let go from a job I had been at for around 2 years because I, like Michael in the story, constantly pushed for us to refactor our crufty legacy code. Our application was a public facing website that received several thousand hits per day, and often would crash 4-5 times a day on average. We actually had three developers leave in three weeks because our ideas were shot down. I was the "last man standing" and got blindsided after buying into the lies I was being fed about fixing things and being promoted to lead.

    2. A week after I left, I heard through the grapevine that they hired someone who took one look at the code and quit, saying he expected them to have better standards. I think he was there a day, if that.

    Sadly, places like that never learn. The longtime developers tend to get lazy and complacent and jealous of any new person who brings in outside ideas (for fear of being shown up) and invariably the new person gets fed yo and quits or is fired because they alone want to improve. Meanwhile, the company continues to pump out crap, blissfully unaware of anything wrong.

  • iMortalitySX (unregistered) in reply to Ama
    Ama:
    The code was working, they had other priorities to code first. Where's the WTF?

    Just to quote the most quoted post here today... Anyways, the real WTF is the future. I worked on a legacy applicaiton which had some great code that was very similar to this. Then one day the customer came out and said that they needed everything moved to a server and the client applicaiton should talk to the server over a secure channel. WCF is awesome, but trying to tear appart the old code to hook in your new code when there are no kidding 100's of different bad ways to do it. That was no fun at all, and took nearly 3x the amount of time it should have to implement the new service.

    [image]
  • Chelloveck (unregistered) in reply to Steven Seagal's ponytail
    Steven Seagal's ponytail:
    Having worked in a "legacy" environment, this was exactly the mindset that I fell into. However, the old dinosaurs would jump all over my code for using newer features of the language because they didn't understand it. I think I still have scars from some of those battles.

    Heh. I was once told by an older programmer not to use the C ternary operator because "no one understands it". You know, the one that goes:

    foo = (bar > 0) ? baz : qux;

    Yeah, there's a brainteaser for you...

  • Noob (unregistered)

    I really, really hate when the new guy feels the need to point out the obvious and then expects a big pat on the back.

    Yeah - gee - nobody here thought that refactoring messy code would be a good idea. It's not that there isn't a business case or other priorities or anything; we just never thought of it. You must be super good.

    Every place I've ever been at could benefit from...

    • Additional time to refactor
    • Additional testing
    • Additional documentation
    • Additional automation

    It's trivial to walk into a place, spend a day observing, and point out the obvious.

  • (cs)

    Ok, we don't know the language for certain but it could be C++. Let's try a macro. We'll need to use token pasting (##) to paste Int String or Float to the end.

    Let's get rid of the counting by introducing a number that gets incremented. For the purpose of sequence points we should increment in a separate statement from the main one.

    #define STMT_INSERT_FROM_RSSOURCE(type,var) \
  stmtInsertToTarget.set##type(var, rsSource.get##type(var)); \
  ++var

    then in our code

    while( rsSource.next() ) {
   int field = 1; 
   STMT_INSERT_FROM_RESOURCE(Int,field);
   STMT_INSERT_FROM_RESOURCE(Int,field);
   STMT_INSERT_FROM_RESOURCE(String,field);
// ...
   stmtInsertToTarget.addBatch();
}

    Ok. Of course I started with a solution I don't like... I was bound not to start with what I really think is a good solution.

    Now a "proper" solution.

    template< typename TARGET, typename SOURCE >
void copyBatch( TARGET & target, SOURCE const& source, 
  const char * schema )
{
   for( size_t index = 1; *schema; ++index ) 
   {
      switch( *schema )
      {
       case 'I': case 'i':
           target.setInt( index, source.getInt( index ) );
           break;

       case 'S': case 's':
           target.setString( index, source.getString( index ) );
            break;
       case 'F': case 'f':
           target.setFloat( index, source.getFloat( index ) );
         break;
       case 'D': case 'd':
            target.setDate( index, source.getDate( index ) );
          break;
       default:
          assert( !"Unsupported type" );
      }
      target.addBatch();
}

    Of course I made it a template here because I don't know the types but you don't have to make it one.

    but look at the comment:

    This section should be enhanced at some point // using a ResultSetMetadata object, iterating // through the column types -aj

    So not even my template solution but a proper object that holds the data was their plan.

  • Vlad Patryshev (unregistered)

    "Don't change anything" is what I heard from all kinds of scared managers. "We don't have time for unittests" is another motto.

    And no, it's not about "gray-haired" people; half of them are half my age.

    Their scope is short. In half a year, when someone will have to go fix/add functionality/change according to new SOX/SOPA/PIPA laws, the manager will either be in another position or happy to ask for more people to fix the mess.

    Yes, it's low morale.

    I don't think this can be fixed by talking to people. These days everybody knows about stuff; if they don't care - quit.

    (and the captcha is "dolor")

  • (cs) in reply to Noob
    Noob:
    I really, really hate when the new guy feels the need to point out the obvious and then expects a big pat on the back.

    Yeah - gee - nobody here thought that refactoring messy code would be a good idea. It's not that there isn't a business case or other priorities or anything; we just never thought of it. You must be super good.

    Every place I've ever been at could benefit from...

    • Additional time to refactor
    • Additional testing
    • Additional documentation
    • Additional automation

    It's trivial to walk into a place, spend a day observing, and point out the obvious.

    The reason why is good companies don't need to be told, and don't need to come up with excuses why they can't do basic housekeeping. Anyone clued in and skilled knows that you refactor code mercilessly and avoid ever getting into the situation where your code is shit and you need to rewrite/refactor it, but can't find the time.

    Any place that gets into that situation has a fundamental issue.

    Addendum (2012-11-20 10:40): Also, good developers know the value of testing and the benefits of using new/emerging technology to solve problems. One doesn't have to be a bandwagon jumper, but good developers don't get into situations like this, and don't work for companies like this.

    All these places ever get are the desperate (who leave ASAP for greener pastures) and the dregs who are unskilled and unaware of it. Anyone smart sees through these places and don't stick around.

  • TheStandardsAreTooDamnHighParty (unregistered)

    You should take a look at the wives of those graybeards....

    I'll bet he'll say:

    "It's a woman okay?" "I know you might prefer one that isn't repulsive, but I need to feed the kids that I created with her ( in error )"

  • (cs)
    "Listen, the app works, OK?" started one of the high ranking senior devs

    The obvious WTF is developers locked so hard into a paradigm that they are afraid to change the code. Code smell!

  • Jasper (unregistered)

    I've seen code like that before in real applications.

    That's what you get when you program with the JDBC API, instead of using an ORM.

  • (cs)

    It's not too dissimilar at our place to be honest. Developers who can't be bothered to use "go to implementation" in VS telling others who can that "interfaces are too hard" and shouldn't be used.

  • (cs)

    I've been landed with a codebase that's at least 16 years old (and probably older). The application has been "upgraded" over the years and currently uses MFC in a VS2008 environment but it has never been "redesigned". New features and fixes has been built on top of other features and bugfixes to the point where the code is nothing short of a complete and utter mess. Not only that, but there's all sorts of "Coding 101 DO NOT DO's" littered through it - global variables for nearly everything, goto statements, you name it. The code is spread through about 40 CPP files and when I say "spread through", I mean it - the "main" function jumps around calling functions from all over the place which also call functions from all over the place. Everything is dependant on everything else. Oh and despite using MFC, the main codebase doesn't use a single class for anything - it's straight up bastardised C with a few scatterings of C++ thrown in at random (there's at least 3 different types of string in use, sometimes within the same function).

    It sounds like a nightmare and, sometimes, it is but miraculously - it WORKS. It's fairly stable in that it only tends to break with something new has been added. I could go in and clean it up, but it would take months for just one developer to do (this is a very small company). It would be much better to just rewrite the app from scratch, but as the app has essentially lived an entirely iterative life, there's no design plan, there's very little code documentation and every little cast or "toupper" is important but nobody knows why (either they can't remember or the developer that wrote it is long gone).

    This kind of thing isn't unique, quite a lot of developers are in the same boat. There really is a legitimate reason why some people say "if it isn't broke, don't fix it".

  • Gaza Rullz (unregistered) in reply to iMortalitySX

    Well that's what you're paid for buddy. :)

  • (cs) 
    //stmtInsertToTarget.setInt(1, 9999);// for testing purposes

    TRWTF are magic numbers for testing!

  • (cs) in reply to Cbuttius

    Can we ban anyone who uses the word "whilst"?

  • (cs) in reply to Chelloveck
    Chelloveck:
    Heh. I was once told by an older programmer not to use the C ternary operator because "no one understands it".
    If you don't mind registering, we're having a nice flame war in the side bar about this.
  • Jack (unregistered) in reply to Cbuttius
    Cbuttius:
    By the way, it is a huge issue I have with this site that you cannot see the article whilst you are writing your comment (unless you open another browser window, of course).
    Oh, dear! If only someone would come up with some technology that would let us see, or do, two things at once! This is such a severe impediment to so many kinds of work -- it is a problem that goes far beyond a single web site and in fact should probably be solved at the OS level. Some kind of windowing system would be awesome.

    I should probably go patent that.

  • (cs) in reply to Chelloveck
    Chelloveck:
    Steven Seagal's ponytail:
    Having worked in a "legacy" environment, this was exactly the mindset that I fell into. However, the old dinosaurs would jump all over my code for using newer features of the language because they didn't understand it. I think I still have scars from some of those battles.

    Heh. I was once told by an older programmer not to use the C ternary operator because "no one understands it". You know, the one that goes:

    foo = (bar > 0) ? baz : qux ~ fnf;

    Yeah, there's a brainteaser for you...

    FTFY to handle the FILE_NOT_FOUND case.

  • (cs) in reply to Jack
    Jack:
    Cbuttius:
    By the way, it is a huge issue I have with this site that you cannot see the article whilst you are writing your comment (unless you open another browser window, of course).
    Oh, dear! If only someone would come up with some technology that would let us see, or do, two things at once! This is such a severe impediment to so many kinds of work -- it is a problem that goes far beyond a single web site and in fact should probably be solved at the OS level. Some kind of windowing system would be awesome.

    I should probably go patent that.

    It's called a scrollbar.

  • Gadfly (unregistered) in reply to Ama
    Ama:
    The code was working, they had other priorities to code first. Where's the WTF?

    The article that I just read was about a guy who told the hiring manager "I hate working inside of sloppy code" and was the hired, only to find that his new job would be "working inside of sloppy code".

    Is it so hard to see the WTF in that? Do you think the hiring manager could have at least warned Michael?

  • Mike (unregistered) in reply to PedanticCurmudgeon
    PedanticCurmudgeon:
    Can we ban anyone who uses the word "whilst"?

    Certainly, consider yourself banned!

  • Mr.Bob (unregistered) in reply to Noob
    Noob:
    I really, really hate when the new guy feels the need to point out the obvious and then expects a big pat on the back.

    Yeah - gee - nobody here thought that refactoring messy code would be a good idea. It's not that there isn't a business case or other priorities or anything; we just never thought of it. You must be super good.

    Every place I've ever been at could benefit from...

    • Additional time to refactor
    • Additional testing
    • Additional documentation
    • Additional automation

    It's trivial to walk into a place, spend a day observing, and point out the obvious.

    Amen to this! I one got some sage advice from a previous coworker who passed on this advice: For the first six months of a new job, Just Shut Up.

    No one likes the new guy coming in and pointing out the obvious, as if all the existing developers are morons. The new guy just doesn't have the feel of the place, or the political capital, to start suggesting changes like that.

  • (cs) in reply to Mike

    Some good comments. The approach I require developers to follow:

    1. NO modifications unless they are directly related to the specific task.

    2. Identify ALL opportunities for code improvements.

    3. IF a piece of code requires modification (see #2), then all pending improvements (see #2) shall be reviewed and determined if they are appropriate to "roll into" the current change.

    This reduces the number of accidental bugs, prevents work-creep (similar to scope creep) and focus the effort on code that is actually important.

  • OldCoder (unregistered) in reply to Cbuttius
    Cbuttius:
    Jack:
    Cbuttius:
    By the way, it is a huge issue I have with this site that you cannot see the article whilst you are writing your comment (unless you open another browser window, of course).
    Oh, dear! If only someone would come up with some technology that would let us see, or do, two things at once! This is such a severe impediment to so many kinds of work -- it is a problem that goes far beyond a single web site and in fact should probably be solved at the OS level. Some kind of windowing system would be awesome.

    I should probably go patent that.

    It's called a scrollbar.

    Unless it's Windows 8.

  • (cs) in reply to Mike
    Mike:
    Certainly, consider yourself banned!
    Well, so are you now since you quoted it.

Leave a comment on “Your Standards Are Too High”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article