Do You Have Scary Code?

  • 30 years ago I rewrote the monthly payroll for a client who was waiting for some real work to come in and wanted to keep hold of me (I was contract). He flung the green striped paper (remember that?) for the main payroll program at me, covered in highlighter and pencil notes, and said it was £500 out every month and could I fix it?

    I spent a happy month or so re-factoring, and then had to abandon that when the real work arrived. I was gutted I hadn't found every last penny, but his comment was I'd got the error down from £500 to£50 and that was an improvement. Line count was 23% down as well, and he was strangely pleased.

  • kate.fletcher 80760 (11/10/2015)


    30 years ago I rewrote the monthly payroll for a client who was waiting for some real work to come in and wanted to keep hold of me (I was contract). He flung the green striped paper (remember that?) for the main payroll program at me, covered in highlighter and pencil notes, and said it was £500 out every month and could I fix it?

    I spent a happy month or so re-factoring, and then had to abandon that when the real work arrived. I was gutted I hadn't found every last penny, but his comment was I'd got the error down from £500 to£50 and that was an improvement. Line count was 23% down as well, and he was strangely pleased.

    It always surprises me when the level of user satisfaction is higher than expected based upon the results.

    Gaz

    -- Stop your grinnin' and drop your linen...they're everywhere!!!

  • I think it's worth distinguishing between code that is scary and complex by necessity vs code that as the saying goes give a thousand monkeys a thousands years in front of keyboards and..... the latter is code I'm generally very wary of touching because it often has no real overall design and generally looks like it was only gotten into a somewhat working state through trial and error.

  • ZZartin (11/10/2015)


    I think it's worth distinguishing between code that is scary and complex by necessity vs code that as the saying goes give a thousand monkeys a thousands years in front of keyboards and..... the latter is code I'm generally very wary of touching because it often has no real overall design and generally looks like it was only gotten into a somewhat working state through trial and error.

    Exactly.

    Regardless of how much time you spend trying to understand what it does, the probability of missing something is very high.

    I'm going through that now trying to clean up a data scrub process that has evolved over the years, and has been touched by many different people.

    We gave up and decided to start from the requirements gathering phase! At least we will have documentation to follow moving forward!

    Michael L John
    If you assassinate a DBA, would you pull a trigger?
    To properly post on a forum:
    http://www.sqlservercentral.com/articles/61537/

  • Gary Varga (11/10/2015)


    Recent client has 6 millions line of code in COBOL, VB5 and PL/SQL (plus a little VB.NET and C# in the mix). It was written in the mid-90s.

    Their opinion is that if you can get away with it then the preference is to leave it alone.

    I don't argue with that.

    It's not that you shouldn't leave it if you can, but you can't be afraid to change it if you need to.

    I'm not advocating change for change's sake. I think if things work, leave them. I have no issue with people running SQL Server 2000 if it works. Heck, if it's running, it's been working.

    But you can't be unwilling to touch code if change is needed.

  • GilaMonster (11/10/2015)


    I've recently seen a system with many user-defined functions, each with thousands of lines of code, often calling each other. Tests or no tests, I'm not touching that.

    Write a test. You might be surprised how much it helps.

    Then write another.

  • Iwas Bornready (11/10/2015)


    Over the years I think I've written some of that scary code. If something needs changing then change it. But I am not a proponent of just changing code because you think you can make it better, because you don't like the way it was written. Half the time that introduces errors. Just leave it alone unless there is a good reason to be in there messing with it.

    Overall I agree with this, but there's an art there. Sometimes changing the code to make it easier to read or simpler is good. However you better not break things, and you shouldn't be wasting time.

    Here's a good look at some reasons why you might want to change code. It's long, but it's really interesting.

    https://www.youtube.com/watch?v=aWiwDdx_rdo

  • Code where it is how to figure out how it works is not (per se) scary. Truly scary code is code where you don't know what it needs to accomplish. If you don't know what the code is supposed to do, it is hard to know what an adequate covering set of tests would be.

    As an example, supposed you find some RBAR code that ends up setting a column in a table to a certain value, based on convoluted logic dependent on other values in the table.

    You very proudly rewrite it to be set based and simplify the logic so that all the column values are set exactly as they had been by the original code (per your unit tests).

    The system is horribly broken when your changes are deployed. What happened?

    It turns out that timing was important, and your new code is too fast, causing terrible problems downstream.

    After a million years (more or less) of analysis of what went wrong, you discover the timing issue and (resignedly) add in a fixed delay. The system works (for a while).

    But then it starts breaking again. After billions of years of further analysis, in which you have memorized every one of the million plus lines of code, you discover that the amount of time that you need to delay by is proportional to this-or-that. As it happens, the original RBAR code had the characteristic that the time it took to run was proportional to this-or-that.

    Truly scary code cannot be addressed by writing unit tests, because the thing that is scary about it is that you don't know what it is supposed to accomplish. It is "doing" many things, which may (or may not) be obvious, and any one of those things it is doing may be important, or may be an unintended artifact of how it was implemented. [In some case, what the code DOESN'T do is as important as what it does do. E.G. It doesn't make use of other obviously relevant records because of contention issues that that might cause.]

    The only solution is to understand the system around the scary code. Sadly, for large old systems, this is often too expensive, and the "best" approach (of a bad lot) is to leave the scary code alone or; when absolutely forced to work on it, do as little as humanly possible. [Another characteristic that is common (though not universal) of scary code in large old systems is that it handles very rare and obscure cases that are nevertheless critical if they are not handled properly.]

  • I completely understand those developers you spoke of who were afraid of modifying some VB6 code. I've known of some code I was afraid of modifying as well. Perhaps a large part of it was that I wasn't even aware of unit testing until maybe 2 years ago. And I'm guessing when I had to take over that awful code (about 10 years ago) there wasn't any unit testing frameworks for VB6. The code was so poorly written with no documentation nor comments (although there were sometimes comments which had just a number put in for God only knows what) that I eventually just re-wrote each piece of code. It was easier than trying to figure out what in heck the original developer was thinking.

    Now though, I wonder if I'd approach it differently. I went to one of the links you gave you gave in your article on Wikipedia listing unit testing frameworks. I fully didn't expect there to be any for VB6. Imagine my surprise when I saw some there. Most were commercial and I'm certain that my managers would not spend a dime on anything like that. But at least one of them is not commercial. Well, that job and that problem is now in my rear view mirror so its not my problem; but I do wonder...

    Kindest Regards, Rod Connect with me on LinkedIn.

  • There is a difference between extremely reluctant and unwilling.

    Sometimes the best solution is to do (lots more) work elsewhere so you don't need to deal with a potentially unstable and intractable mass of scary code. I.E., I can make the change that I *think* I need in the scary code in 10 minutes, but I *might* be spending the next 3 weeks cleaning it up and trying to things that broke as a result.

  • Steve Jones - SSC Editor (11/10/2015)


    Gary Varga (11/10/2015)


    Recent client has 6 millions line of code in COBOL, VB5 and PL/SQL (plus a little VB.NET and C# in the mix). It was written in the mid-90s.

    Their opinion is that if you can get away with it then the preference is to leave it alone.

    I don't argue with that.

    It's not that you shouldn't leave it if you can, but you can't be afraid to change it if you need to.

    I'm not advocating change for change's sake. I think if things work, leave them. I have no issue with people running SQL Server 2000 if it works. Heck, if it's running, it's been working.

    But you can't be unwilling to touch code if change is needed

    Absolutely.

    Gaz

    -- Stop your grinnin' and drop your linen...they're everywhere!!!

  • Rod at work (11/10/2015)


    ...I wasn't even aware of unit testing until maybe 2 years ago. And I'm guessing when I had to take over that awful code (about 10 years ago) there wasn't any unit testing frameworks for VB6...

    Many of us were coding unit tests long before we were aware of frameworks or automated unit tests. Occasionally we all find ourselves ahead of a curve - with hindsight 😉

    As my graphics lecturer always said: "I hate laziness unless it is efficient laziness!!!" i.e. doing something as a shortcut is wrong but doing a pragmatic approximation is most excellent.

    Gaz

    -- Stop your grinnin' and drop your linen...they're everywhere!!!

  • GilaMonster (11/10/2015)


    I've recently seen a system with many user-defined functions, each with thousands of lines of code, often calling each other. Tests or no tests, I'm not touching that.

    They've done the same with views where I work. Views calling views calling views and they all call the same set of tables. The only saving grace is there are no aggregates that they can join on. Still, they're a joined nightmare that basically resolve to one of those huge single queries with multiple self-joins and the occasional many-to-many join that they've overcome the results of with DISTINCT.

    Oddly enough, though, they're not the worst problem that I'm currently or have been concerned with. To give you the idea of the type of code I've been repairing (you've been there, for sure), the code was written by a revolving door of about 15-20 different "developers" based on a database designed and implemented by those same people and they had about 4-5 years with no other guidance except "get 'er done". After 4 years, we're still cleaning up the mess.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.
    "Change is inevitable... change for the better is not".

    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)
    Intro to Tally Tables and Functions

  • Iwas Bornready (11/10/2015)


    Over the years I think I've written some of that scary code. If something needs changing then change it. But I am not a proponent of just changing code because you think you can make it better, because you don't like the way it was written. Half the time that introduces errors. Just leave it alone unless there is a good reason to be in there messing with it.

    +1 If it ain't broke...

    Don Simpson



    I'm not sure about Heisenberg.

  • A foolish man builds his business on top of a million lines of source code.

    "Do not seek to follow in the footsteps of the wise. Instead, seek what they sought." - Matsuo Basho

Viewing 15 posts - 16 through 30 (of 69 total)

You must be logged in to reply to this topic. Login to reply