• MiserableOldGit (unregistered)

    Oh, I'll say the FIRST one

  • MiserableOldGit (unregistered)

    and this is only a WTF if the coder in question had proper control over all the other bits of code that may have called that function. I've done that when I either don't fully know or can't change other bits of the system and don't want to cause a hard crash in production.

    I'd put a comment, though, and firmly resolve to go back and fix it properly some day (yeah, right).

  • (nodebb)

    And I thought my codebase was bad. At least the dead code would be commented out.

  • just me (unregistered) in reply to MiserableOldGit

    I'm curious: how would removing the dead code and replacing it with a comment along the lines of "This was really complex in the past but new requirement #xxx means that we don't need that any more because yyy" have caused hard crashes in production?

  • copy-and-paste-programmer-hater (unregistered)

    Look at that: if(container.widgets[i].settings.dateRange.relativeDate){ if (container.widgets[i].settings.dateRange.relativeDate === '-1y') ...

    Why not assign a local variable instead? var date = container.widgets[i].settings.dateRange.relativeDate; if(date ){ if (date === '-1y') ...

    The 2 excuses of these copy-and-paste programmers:

    Excuse 1: It's faster for me: the first version takes me 10 seconds to type with IDE-autocomplete on, but the second version takes 12 seconds for me. My answer: Do you know that code is written once, but read 10 times in average? You save 2 seconds, but the company must pay for additional minutes caused by readers because it's harder to understand.

    Excuse 2: It doesn't run slower, the compiler will see the duplication and inline it. My answer: The compiler is not allowed to. If you have a method getX() and call it 2 times then the compiler does not know if the class has changed the value in the meantime (like a time counter or iterator would do), or if the value is the same.

    Excuse 3: I don't care about time, it just runs the same way, and important for me is that the first if-statement guards against a crash. My answer: No. The first version may crash inside the if-statement body whereas the second cannot. Your check for it is meaningless, because you try to access the value again which could become null in the meantime. Java-Snippet: if (a.getX() != null) { System.out.print(a.getX().getY()) } So when it goes inside the if-statement, another thread can interrupt and call a.setX(null). Then the thread continues and when accessing a.getX().get(Y) throws a NullPointerException!

  • MiserableOldGit (unregistered) in reply to just me

    No that would be fine, What I meant was deleting the entire function as it's clearly redundant/superfluous.

  • Anon Derpinsky (unregistered) in reply to MiserableOldGit

    Remember, this is TDWTF.

    all the 27492 compiler errors complaining about a call to an undefined function all that regactoring all those classes/methods/what ever that use a completely different function but with the same name from a different header

  • WizardStan (unregistered)

    For those that missed it, because I know I did on several passes trying to figure out what, exactly, was wrong with the function: the first line is "return true;" rendering everything after it useless.

  • C0wboy (unregistered)

    I missed it at first. Sort of reminds me where I worked once, where although we used source control, for every change we were required to comment out the old code, not delete it. Additionally, we would add a comment to the code with the change number. Predictably, this led to a huge mess. To make matter more enjoyable, although we were developing on Windows, we used an MS-DOS client for the source control, which was very painful. Procedures also required that we have a word document for every change that was saved in a specific folder.

    This was my first development job, I was quite worried at the time that I had made a dreadful mistake in my choice of career.

  • MiserableOldGit (unregistered) in reply to C0wboy

    Wasn't a certain well known travel firm was it?

  • C0wboy (unregistered) in reply to MiserableOldGit

    Afraid not. They made banking software - I will say no more. Sadly this was before TDWTF - there would have been numerous CodeSOD candidates.

  • No, Your Name (unregistered)

    You get the first set of requirements and write up the logic for them; then the second set come in and it's easier to scrap the first set and write it from scratch; then the third set come in and they're almost the same as the first; by this time you've decided not to delete code but to keep it around for when the mood swings back next week and it's needed again.

    Sure, adding comments around each version would help, but it's commonplace to do this.

  • My Name (unregistered) in reply to MiserableOldGit

    no, it is frist line!

  • (nodebb)

    That reminds me: I once spent 4 hours analyzing 1100 lines of COBOL source, which ultimately computed just one variable...which was zeroed immediately following the code block.

  • Derf Skren (unregistered) in reply to WizardStan

    Yeah if people used whitespace, in this case a blank line under the "return true", properly then this sort of thing would be much easier to spot.

  • Gerry (unregistered)

    As there is no source history, I would be worried that someone had added the "return true;" to enable them to test something more easily, and forgot to reinstate the tests. S smart dev might use an #IFDEF DEBUG or language equivalent, but a smart dev wouldn't be working in such a place.

  • Derek (unregistered)

    I once had some code like this in one of my experimental development branches:

    const int numberOfTimesBobChangedHisMind = 7;

    if (numberOfTimesBobChangedHisMind % 2 == 0) { //... } else { //... }

  • Gene Wirchenko (unregistered) in reply to MiserableOldGit

    MiserableOldGit, having the call could be useful in the future. I have some class code where a given method is but a return. Sometimes, that is all that is needed, but the hook is there for those subclasses that need something there.

  • (nodebb)

    I see Nagesh has a new job.

  • Greg (unregistered)

    Implementing specs like walking on water: much easier when frozen

  • TheCPUWizard (unregistered)

    Code should not exist in isolation - it is only one part of a complex eco system.

    Why does the organization want to invest in something [Epic]? What is the overall value representation [Feature]? What are the individual value/interactions? [Story/PBI]? Those are the "why" for the code...

    How is the value going to be delivered, what actions need to be taken [Task]? This is the "what" for the code.

    Then small incremental changes [Commits] that are each 100% focused on a single Task, and has a detailed comment bridging the gap of what is known from the task and what is clear from a diff on the code.

    Have this proper ecosystem in place (and this has been around for well over a decade, it is not "new") and such situations are completely avoided.

  • (nodebb)

    Any good linter would throw an error about unreachable code for this... I guess they're not linting their code.

Leave a comment on “Changing Requirements”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article