The Daily WTF: Curious Perversions in Information Technology
Welcome to TDWTF Forums Sign in | Join | Help
in Search

It doesn't need refactoring

Last post 04-02-2012 1:41 PM by serguey123. 35 replies.
Page 1 of 1 (36 items)
Sort Posts: Previous Next
  • 03-26-2012 2:34 PM

    It doesn't need refactoring

    A peer has been off to the side working on something big in a separate branch - for several months. Yesterday, he merged into the main trunk, and I got the, erm, privilege, of reviewing what he did.

    BrandOneProduct1Event.java
    BrandOneProduct1EventDAO.java // one of these loads reference data
    BrandOneProduct1EventsDAO.java // the other one saves new records

    BrandOneProduct2Event.java
    BrandOneProduct2EventDAO.java // one of these loads reference data
    BrandOneProduct2EventsDAO.java // the other one saves new records
    ...
    BrandOneProduct14Event.java
    BrandOneProduct14EventDAO.java // one of these loads reference data
    BrandOneProduct14EventsDAO.java // the other one saves new records
    ...
    BrandSixProduct14Event.java
    BrandSixProduct14EventDAO.java // one of these loads reference data
    BrandSixProduct14EventsDAO.java // the other one saves new records

    Of course, he simply refused to put the load and save code for a given type of object in the same class, so you just need to randomly open one of XxyEventDAO and XyzEventsDAO to figure out which one does what.

    Also, Brands 1-6 are nearly identical, save for a few fields, so how about a base class with subclasses adding only what's needed? That's actually what he did in the DB; a primary table, with ancillary tables adding only the separate fields for each object. But not in code, no. Each class is a stand alone base class with all the common members, accessors and utility methods duplicated - across 84 classes.

    When I told him this was a pointless maintenance nightmare when the inevitable changes came along, he claimed that it made sense to him, and that it doesn't need refactoring.

    I'll let my boss discuss it with him.

     

  • 03-26-2012 2:41 PM In reply to

    Re: It doesn't need refactoring

    Any class name with an ordinal in it is a WTF.

    Let the healing begin!

  • I may not agree with everything Morbs just said, but he expresses himself in a way that is dignified, respectful, polite and non-threatening!
  • 03-26-2012 2:46 PM In reply to

    Re: It doesn't need refactoring

    morbiuswilters:
    Any class name with an ordinal in it is a WTF.
    The brand/product class names are anonymized, but the person who wrote this is a walking WTF, straight off the boat of cheap H1-B imported labor.

  • 03-26-2012 4:37 PM In reply to

    Re: It doesn't need refactoring

    snoofle:

    ... he claimed that it made sense to him, and that it doesn't need refactoring ...

    Quite right. It doesn't need refactoring; it needs reauthoring, preferably by a different author.

    snoofle:

    A peer ...

    Ah, this is obviously some strange usage of the word 'peer' that I wasn't previously aware of.

    Peace, unless you're a cow.
  • 03-26-2012 6:18 PM In reply to

    Re: It doesn't need refactoring

    snoofle:

    he claimed that it made sense to him, and that it doesn't need refactoring.

    I'll let my boss discuss it with him.

     

    We had a coworker like that. Everything he did made sense to him.. unfortunately it didn't make sense to anyone else which lead to a lot of heated discussions with the software dev manager. The end result of any of those discussions was simply: just do it the way we told you to.

    Any discussions about architecture I'd make sure to have in ear shot of that manager so I could short-cut having to refactor/rewrite and just pull a, "What do you think $boss?" As he would make sure to keep an open ear on such discussions to try an nip the WTFery in the bud.

    This guy was a piece of work though. He once read that functions should be no more than a few lines (or something along those lines), which he bought into wholesale (no clue where he read that). So I never had to worry about 100 line methods. Instead I had to worry about digging through a depth of 9 calls across 4 classes to find what a call did. A call the could have just been a win32 call at the top level (single frigging function call). Each depth would add another argument until you had all the arguments from various objects at the final call.

    Not an original name, but I like to call him "the complicator". If something could be done in 3 steps: A->B->C, instead of writing it that way, or going directly A->C he would do something along the lines of: 01->A1->2->D->F->B1->B2->B3->E->A3->C

    We called it lily-padding the solution (i.e. trying to cross a body of water by jumping on lily pads, with 0 planning before hand). He'd eventually get there, but he often did a lot of backtracking and unnecessary work in the effort. Any other dev would have used the bridge that was already constructed.

    This sounds like a simple day or two refactor. I don't see why he's opposed other than being stubborn. I could see a, "Yeah, yeah, once I did the data layer I was a lot clearer about it. Didn't refactor due to time." But outright saying it doesn't need a refactor? Come on, his own code proves it.
  • 03-26-2012 6:58 PM In reply to

    Re: It doesn't need refactoring

    dubbreak:
    This sounds like a simple day or two refactor. I don't see why he's opposed other than being stubborn. I could see a, "Yeah, yeah, once I did the data layer I was a lot clearer about it. Didn't refactor due to time." But outright saying it doesn't need a refactor? Come on, his own code proves it.
    Most times I've seen this, the answer has turned out to be that copy and paste is less taxing on the poor soul than thinking about the problem in a reasonable fashion.
    I denounce myself for this post
  • 03-26-2012 7:05 PM In reply to

    • PJH
    • Top 10 Contributor
    • Joined on 02-14-2007
    • Newcastle, UK
    • Posts 3,919

    Re: It doesn't need refactoring

    dubbreak:
    This guy was a piece of work though. He once read that functions should be no more than a few lines (or something along those lines), which he bought into wholesale (no clue where he read that).
    Because there are idiotic coding standards enforcing such limits and idiotic programmers suggesting such limits littered all over the place.
    "Because you watched 'The Very Hungry Caterpillar,' we recommend 'The Human Centipede.'"
    --
    UED - Countryside: To kill Piers Morgan
  • Parp!
  • 03-26-2012 7:50 PM In reply to

    Re: It doesn't need refactoring

    PJH:
    dubbreak:
    This guy was a piece of work though. He once read that functions should be no more than a few lines (or something along those lines), which he bought into wholesale (no clue where he read that).
    Because there are idiotic coding standards enforcing such limits and idiotic programmers suggesting such limits littered all over the place.
    I would be more impressed if you had found a link to TDWTF.

    Let the healing begin!

  • I may not agree with everything Morbs just said, but he expresses himself in a way that is dignified, respectful, polite and non-threatening!
  • 03-26-2012 8:04 PM In reply to

    • PJH
    • Top 10 Contributor
    • Joined on 02-14-2007
    • Newcastle, UK
    • Posts 3,919

    Re: It doesn't need refactoring

    morbiuswilters:
    I would be more impressed if you had found a link to TDWTF.
    The nearest my google-fu could get to it is this one asking about limits on function length in Adobe Flash back in 2008.
    "Because you watched 'The Very Hungry Caterpillar,' we recommend 'The Human Centipede.'"
    --
    UED - Countryside: To kill Piers Morgan
  • Parp!
  • 03-26-2012 8:22 PM In reply to

    Re: It doesn't need refactoring

    PJH:
    morbiuswilters:
    I would be more impressed if you had found a link to TDWTF.
    The nearest my google-fu could get to it is this one asking about limits on function length in Adobe Flash back in 2008.
    Here's something sort of relevant from discussion about the calculator contest:
    DAL1978:
    My best/worst function has to be my division. Note that I am storing and processing numbers as strings with the order of the digits backwards. I have managed to come up with a valid use of true/false/maybe and abuse logic by gotoing from the if to the else block. My second best function is the subtraction which I refactored from 2 medium length functions into 144 functions of around 5 lines each.
    I denounce myself for this post
  • 03-26-2012 8:51 PM In reply to

    Re: It doesn't need refactoring

     It's true that if a function gets very long (many unix-type projects put that level at the standard text editor's screen size), you should consider refactoring it into multiple /logical/ steps. Of course, there are the types that simple draw a line half way through the function, and make a _2 that is passed by reference every variable in play. Has anyone seen any such orangutan try to split a function in the middle of a loop?

    Extracted from EM radiation remaining from the end of previous universe -
    //Quantum Bogosort.cpp - Implement Quantum bogosort Algorythym
    // (c) Jamie Bean
    // Dedicated to my brillant mum, Paula
  • 03-26-2012 9:04 PM In reply to

    Re: It doesn't need refactoring

    PJH:
    dubbreak:
    This guy was a piece of work though. He once read that functions should be no more than a few lines (or something along those lines), which he bought into wholesale (no clue where he read that).
    Because there are idiotic coding standards enforcing such limits and idiotic programmers suggesting such limits littered all over the place.
    If this guy did functions that were 20 lines long I would have been happier. Somehow he got the idea that 1 line was the length to shoot for. And when he got on that stint the longest I saw was around 5 lines.

    This points towards the overall problem of people that can't use sense and must follow rules. Does extracting the portion of that long function as a separate function/procedure/method make sense? Does it make it more readable? Is it a good abstraction? Is it providing legitimate re-usability? I mean, think about what you are doing for FSM's sake.

    Why was the suggestion of 20 lines made? To be more readable? Ok, does splitting up that function make it more or less readable? THINK! As a dev that's what you're paid to do. If you want to just program then be prepared to be told what to do by someone else. Don't like that? Then use your brain.

    That guy was let go. The mess he left wasn't fully understood until he left. Somehow, absurd um-maintainable coding practices aside, he wrote stable code that was amazingly bug free (considering his practices). Of course I'd take maintaining the code from the bug prone guy that only coded for the ideal conditions as he commented and had a reasonable clue on how to structure code. Refactoring his code was livable as opposed to a living nightmare. This guy LOVED inheritance. He tried to incorporate it wherever possible, because.. well that's what you do in OOP right? I have never seen such weird abuses of polymorphism before. I really should have kept some to submit, but none could be capture in a few lines anyhow. The call 9 layers deep with one or two lines per function was a good one. Should have submitted that.
  • 03-26-2012 9:51 PM In reply to

    • PJH
    • Top 10 Contributor
    • Joined on 02-14-2007
    • Newcastle, UK
    • Posts 3,919

    Re: It doesn't need refactoring

    dubbreak:
    Why was the suggestion of 20 lines made?
    Probably a throwback to when terminals were only 80x25 and the idea was to be able to see the whole function on the screen at the same time in most cases.
    "Because you watched 'The Very Hungry Caterpillar,' we recommend 'The Human Centipede.'"
    --
    UED - Countryside: To kill Piers Morgan
  • Parp!
  • 03-26-2012 10:14 PM In reply to

    Re: It doesn't need refactoring

    PJH:
    dubbreak:
    Why was the suggestion of 20 lines made?
    Probably a throwback to when terminals were only 80x25 and the idea was to be able to see the whole function on the screen at the same time in most cases.
    Because, God knows, it's not possible to understand a function if you can't see the entire thing all at once. And, God knows, nobody has a screen height over 25 lines.

    Let the healing begin!

  • I may not agree with everything Morbs just said, but he expresses himself in a way that is dignified, respectful, polite and non-threatening!
  • 03-26-2012 11:40 PM In reply to

    Re: It doesn't need refactoring

    PJH:
    dubbreak:
    This guy was a piece of work though. He once read that functions should be no more than a few lines (or something along those lines), which he bought into wholesale (no clue where he read that).
    Because there are idiotic coding standards enforcing such limits and idiotic programmers suggesting such limits littered all over the place.

    At the risk of sounding repetative: individuals like structure and guidelines, and in the absence of any they'll either enjoy the freedom to become a loose cannon or adopt a set of practises from elsewhere, believing it to be more beneficial than without.

    (I know in this case the issue revolved around not adhering to organisational policy yet had no problems following external guidance. WTF?)

    Those links are quite interesting - many use "should", "ideally" - implying that they are suggestions and aspirations rather than cold rules. Does anyone here have such rules/guidance in their organisation's coding policies? (I've not heard of it meself)

    morbiuswilters:
    PJH:
    dubbreak:
    Why was the suggestion of 20 lines made?
    Probably a throwback to when terminals were only 80x25 and the idea was to be able to see the whole function on the screen at the same time in most cases.
    Because, God knows, it's not possible to understand a function if you can't see the entire thing all at once. And, God knows, nobody has a screen height over 25 lines.

    Not back in the day of dumb terminals, they didn't. You young 'uns don't know how lucky you are with your colour and sound and mouse thisand drag that, this was all fields, etc.

    Perl has some rule about the number of bugs trippling when the code goes over one page/screen, doesn't it?

  • 03-27-2012 2:20 AM In reply to

    • Weps
    • Not Ranked
    • Joined on 11-06-2008
    • Posts 75

    Re: It doesn't need refactoring

    Cassidy:

    Those links are quite interesting - many use "should", "ideally" - implying that they are suggestions and aspirations rather than cold rules. Does anyone here have such rules/guidance in their organisation's coding policies? (I've not heard of it meself)

     I'ld rather have suggestions in a coding guide than idiotic "must" rules.

  • 03-27-2012 4:33 AM In reply to

    Re: It doesn't need refactoring

     20 lines or less for a method or function? That's a bloody joke.

    It's as bad as the rule (although not always strictly enforced) that code in the Linux kernel shouldn't go beyond 80 characters per line because of old tty terminals. I'm not sure, but I think people may have gone beyond those now to use larger monitors with support for non-monospaced fonts, but maybe that's just me being crazy and assuming too many advances in technology have been adopted on a wider scale?

  • 03-27-2012 8:26 AM In reply to

    • KattMan
    • Top 75 Contributor
    • Joined on 10-18-2006
    • Location Unknown, Destination Anywhere
    • Posts 662

    Re: It doesn't need refactoring

    Personally, I could care less abotu number of lines in a function, that's as stupid as paying a programmer per line of code.

    I much preferr and in some places made ti a strong recommendation when I coudl have the influence that a finction should have it's focus on one task.  If you could do that task in 5 lines or 500 I didn't care, as long as it was a single task.

    Now how do you define tasks, well we did hope our programmers had some common sense and didn't call "processing this entire file of individual transactions while charging the credit cards" or "adding this variable to a string" a single task.  Code reviews kept that at least in the realm of sanity.

    CAPTCHA? We ain't got no CAPTCHA. We don't need no CAPTCHA. We don't need no stinking CAPTCHA!
  • 03-27-2012 8:41 AM In reply to

    Re: It doesn't need refactoring

    KattMan:

    Personally, I could care less abotu number of lines in a function, that's as stupid as paying a programmer per line of code.

    I much preferr and in some places made ti a strong recommendation when I coudl have the influence that a finction should have it's focus on one task.  If you could do that task in 5 lines or 500 I didn't care, as long as it was a single task.

    Now how do you define tasks, well we did hope our programmers had some common sense and didn't call "processing this entire file of individual transactions while charging the credit cards" or "adding this variable to a string" a single task.  Code reviews kept that at least in the realm of sanity.

    But how much less? What if we start with a simple bound that we can compare? More or less than spelling typos?
    I denounce myself for this post
  • 03-27-2012 8:52 AM In reply to

    Re: It doesn't need refactoring

    Weps:
    I'ld rather have suggestions in a coding guide than idiotic "must" rules.

    That was kinda my point - I read them as COULD DO and SHOULD DO, rather than MUST DO.

    (or maybe I misread them. It was early morning and I didn't take it all in)

  • 03-27-2012 11:29 AM In reply to

    Re: It doesn't need refactoring

    Cup1ProductDrinkEvent  // One of these contains wine, one contains poision.

    Cup1ProductDrinksEvent  // The battle of wits has begun.


  • 03-27-2012 11:50 AM In reply to

    Re: It doesn't need refactoring

    snoofle:
    BrandOneProduct1Event.java
    BrandOneProduct1EventDAO.java // one of these loads reference data
    BrandOneProduct1EventsDAO.java // the other one saves new records

    *snip*

    Of course, he simply refused to put the load and save code for a given type of object in the same class, so you just need to randomly open one of XxyEventDAO and XyzEventsDAO to figure out which one does what.

    Was it consistent (eg, Loading in Event and Saving in Events) or random? Please tell me it was random.

  • 03-27-2012 12:01 PM In reply to

    Re: It doesn't need refactoring

    pkmnfrk:
    snoofle:
    BrandOneProduct1Event.java
    BrandOneProduct1EventDAO.java // one of these loads reference data
    BrandOneProduct1EventsDAO.java // the other one saves new records

    *snip*

    Of course, he simply refused to put the load and save code for a given type of object in the same class, so you just need to randomly open one of XxyEventDAO and XyzEventsDAO to figure out which one does what.

    Was it consistent (eg, Loading in Event and Saving in Events) or random? Please tell me it was random.


    I bet it was consistent but he changed his mind part way through (possibly more than once). Don't worry, it makes sense to him.
  • 03-27-2012 1:27 PM In reply to

    Re: It doesn't need refactoring

    pkmnfrk:
    Was it consistent (eg, Loading in Event and Saving in Events) or random? Please tell me it was random.
    It was mostly consistent (three exceptions, and one of them neither loaded nor saved data; it just had seven unrelated functions that the guy just put in there, probably because it was the file he had open when he realized he needed those functions... but that's so typical around here that I don't even bother posting stuff like that)

  • 03-27-2012 7:22 PM In reply to

    Re: It doesn't need refactoring

    RichP:

    Cup1ProductDrinkEvent  // One of these contains wine, one contains poision.

    Cup1ProductDrinksEvent  // The battle of wits has begun.


    2cups1RichP
  • 03-28-2012 9:36 AM In reply to

    Re: It doesn't need refactoring

    morbiuswilters:
    PJH:
    dubbreak:
    Why was the suggestion of 20 lines made?
    Probably a throwback to when terminals were only 80x25 and the idea was to be able to see the whole function on the screen at the same time in most cases.
    Because, God knows, it's not possible to understand a function if you can't see the entire thing all at once. And, God knows, nobody has a screen height over 25 lines.

    Um, I used to have a co-worker (who also happened to be my boss) who was like that. Whether his dyslexia had anything to do with it, I don't know, but he needed to have all of a function on the screen before him. (And this was at the time that 17" screens were the standard for professionals.)

    His solution? Don't use any formatting (after all, C is a context-free language) and have all code fit on one screen.

  • 03-28-2012 9:42 AM In reply to

    Re: It doesn't need refactoring

    CarnivorousHippie:

    snoofle:

    A peer ...

    Ah, this is obviously some strange usage of the word 'peer' that I wasn't previously aware of.

    Who says the guy isn't nobility? Everybody knows that nobility are useless at anything (except hunting poor defenceless animals and nurturing inbred mental retardation), and this includes programming.

     

  • 03-28-2012 10:40 AM In reply to

    Re: It doesn't need refactoring

    PJH:
    dubbreak:
    This guy was a piece of work though. He once read that functions should be no more than a few lines (or something along those lines), which he bought into wholesale (no clue where he read that).
    Because there are idiotic coding standards enforcing such limits and idiotic programmers suggesting such limits littered all over the place.

     I hate you for making me read that first link, which advocates using exceptions to fake goto.

  • 03-28-2012 11:01 AM In reply to

    • PJH
    • Top 10 Contributor
    • Joined on 02-14-2007
    • Newcastle, UK
    • Posts 3,919

    Re: It doesn't need refactoring

    FrostCat:
     I hate you for making me read that first link, which advocates using exceptions to fake goto.
    You bothered reading that far? The bit I was referring to was the last paragraph of the first section

    Anyway, you're welcome.
    "Because you watched 'The Very Hungry Caterpillar,' we recommend 'The Human Centipede.'"
    --
    UED - Countryside: To kill Piers Morgan
  • Parp!
  • 03-28-2012 2:14 PM In reply to

    Re: It doesn't need refactoring

    Severity One:
    after all, C is a context-free language
    No, it's not.

    Let the healing begin!

  • I may not agree with everything Morbs just said, but he expresses himself in a way that is dignified, respectful, polite and non-threatening!
  • 03-28-2012 11:45 PM In reply to

    • Zemm
    • Top 50 Contributor
    • Joined on 11-26-2007
    • Gold Coast, Australia
    • Posts 1,381

    Re: It doesn't need refactoring

    Sutherlands:

    RichP:

    Cup1ProductDrinkEvent  // One of these contains wine, one contains poision.

    Cup1ProductDrinksEvent  // The battle of wits has begun.

    2cups1RichP
     

    As you wish

     

    Filed Under: Can you think of something that talks, other than a person?
  • 03-29-2012 12:19 AM In reply to

    Re: It doesn't need refactoring

    REgarding lines of code in a functions...my general rule is if you can not describe the function in sufficient detail for a person to comprehend the functionallity in one sentance, it *might* be too big.
  • 03-29-2012 2:49 AM In reply to

    Re: It doesn't need refactoring

    TheCPUWizard:
    REgarding lines of code in a functions...my general rule is if you can not describe the function in sufficient detail for a person to comprehend the functionallity in one sentance, it *might* be too big.
     

    If your function's parse tree does not fork...you might be a redneck coder!

  • 04-02-2012 9:36 AM In reply to

    • serguey123
    • Top 25 Contributor
    • Joined on 09-30-2010
    • Candyland (formely known as Corruptsylvania)
    • Posts 1,828

    Re: It doesn't need refactoring

    Severity One:

    CarnivorousHippie:

    snoofle:

    A peer ...

    Ah, this is obviously some strange usage of the word 'peer' that I wasn't previously aware of.

    Who says the guy isn't nobility? Everybody knows that nobility are useless at anything (except hunting poor defenceless animals and nurturing inbred mental retardation), and this includes programming.

     

    Hey! that hurts... btw it seems that this site is chrome retarded
    #100085 +(531)- [X]

    <jome> I suppose there are bad coders, there are really bad coders, there are fucking retards, and there's coders writing lines like this: $resultscode = $resultscode - 0;


  • 04-02-2012 1:24 PM In reply to

    Re: It doesn't need refactoring

    serguey123:
    btw it seems that this site is chrome retarded
    Works fine for me, except for the fact that the editor uses plain mode instead of RTF mode. But since CS hasn't been updated since before Chrome was released, that's understandable.

    Let the healing begin!

  • I may not agree with everything Morbs just said, but he expresses himself in a way that is dignified, respectful, polite and non-threatening!
  • 04-02-2012 1:41 PM In reply to

    • serguey123
    • Top 25 Contributor
    • Joined on 09-30-2010
    • Candyland (formely known as Corruptsylvania)
    • Posts 1,828

    Re: It doesn't need refactoring

    morbiuswilters:
    serguey123:
    btw it seems that this site is chrome retarded
    Works fine for me, except for the fact that the editor uses plain mode instead of RTF mode. But since CS hasn't been updated since before Chrome was released, that's understandable.
    Besides the editor the font seems screwy
    #100085 +(531)- [X]

    <jome> I suppose there are bad coders, there are really bad coders, there are fucking retards, and there's coders writing lines like this: $resultscode = $resultscode - 0;


Page 1 of 1 (36 items)
Powered by Community Server (Non-Commercial Edition), by Telligent Systems