• Cheddar (unregistered)

    Self documenting code. Isn't this what we all should strive for?


    p.s. woohoo 1st post

  • jas (unregistered)

    I believe for the page data snippet that it's 'manual edit time polymorphism' - they can come back in and change one of the switches to do something else (not at runtime, but edit time).  Follows good programming practices (along with cut-n-paste programming).

  • (cs)

    Can anyone say "Copy and Paste"[:^)]

  • (cs)
    <font size="2">//Load Page Data
    LoadPageData();

    now that's good commenting.</font>
  • Dave (unregistered)

    Hey this isn't a classic WTF, I cant find it anywhere.

  • (cs)
    Jake Vinson:
       lblNavigation.text = ddlOperationList.Items.Item(ddlOperationList.SelectedIndex).Text


    Apart from everything else, what's wrong with          

    lblNavigation.text = ddlOperationList.SelectedValue

    to get the text of a drop-down list? Maybe even .ToString it to be extra-sure ...


  • (cs)

    To be honest, this isn't all THAT bad.

    In both cases, one possible explaination is that at one time, different states did different things. However, they were changed such that they would all do the same thing, with the expectation that they would do different things at some time soon in the future, something that never came to pass.

    The first case actually seems more like someone just doesn't understand how switch works, and performed a monkey-see-monkey-do to make the code work.

  • Tom (unregistered)

    Well, if LoadPageData had some sort of way to tell without being passed any information what the page state was, that might make sense...or maybe the programmer was being paid by the line.

  • (cs) in reply to Volmarias

    Volmarias:
    To be honest, this isn't all THAT bad. In both cases, one possible explaination is that at one time, different states did different things. However, they were changed such that they would all do the same thing, with the expectation that they would do different things at some time soon in the future, something that never came to pass. The first case actually seems more like someone just doesn't understand how switch works, and performed a monkey-see-monkey-do to make the code work.

    I think we found the original developer of this app. [;)]

  • (cs)

    "You can paint it any color, so long as it's black" - Henry Ford

  • (cs) in reply to Tom
    Anonymous:
    Well, if LoadPageData had some sort of way to tell without being passed any information what the page state was, that _might_ make sense...or maybe the programmer was being paid by the line.

    Cool!
    Endless Switch-Statements make you rich!

    int s = 0;
    switch(s) {
       0:  //Load Page Data
       
    LoadPageData();
           break;
        1: // Make Money
           dontBotherBreaking();
        2: // Make Money
        ...
        default: // You should allways have a default
                   // Make Money!
    }

    I'd love to live in that world  <:o)


  • (cs) in reply to chb

    <FONT color=#000000 size=2>Well they could have done it this way:</FONT>

    <FONT color=#000000 size=2>Dim i As Integer = 0
    While i < 10
    //Load Page Data
        LoadPageData();
    End While</FONT>

    <FONT size=2>
    <FONT color=#000000>Yes this is a joke[H]</FONT></FONT>

    <FONT color=#0000ff size=2>

    </FONT>
  • (cs) in reply to Ken Nipper
    Ken Nipper:

    <font color="#000000" size="2">Well they could have done it this way:</font>

    <font color="#000000" size="2">Dim i As Integer = 0
    While i < 10
    //Load Page Data
        LoadPageData();
    End While</font>

    <font size="2">
    <font color="#000000">Yes this is a joke[H]</font></font>

    <font color="#0000ff" size="2"></font>


    What's the joke? The fact that you never increment the loop index?
  • (cs) in reply to John Smallberries
    John Smallberries:
    Ken Nipper:

    <FONT color=#000000 size=2>Well they could have done it this way:</FONT>

    <FONT color=#000000 size=2>Dim i As Integer = 0
    While i < 10
    //Load Page Data
        LoadPageData();
    End While</FONT>

    <FONT size=2>
    <FONT color=#000000>Yes this is a joke[H]</FONT></FONT>

    <FONT color=#0000ff size=2></FONT>


    What's the joke? The fact that you never increment the loop index?

    Yes.  Now it will load page data indefinitely just to make sure it gets done[:P]

  • AC (unregistered) in reply to Ken Nipper
    Ken Nipper:

    <font size="2"><font color="#000000">Yes this is a joke[H]</font></font>

    <font color="#0000ff" size="2"></font>



    jokes are funny
  • ldiggity (unregistered) in reply to AC

    weak

  • (cs) in reply to ldiggity
    <!--StartFragment --> "Everyone thinks they have a sense of humor, but then they don't always." - <!--StartFragment --> Steve Martin
  • (cs) in reply to ldiggity

    Actually, the later one isn't too bad, and there are reasons why someone would want to do it like that, like if they want it to do said action if the variable isn't set, or set to Y. It might be that for all other cases (variable set to non-zero, non-Y value) that another action should take place, or this one shouldn't. Still, it could have been condensed onto one line.

  • (cs) in reply to Volmarias

    Volmarias:
    To be honest, this isn't all THAT bad. In both cases, one possible explaination is that at one time, different states did different things. However, they were changed such that they would all do the same thing, with the expectation that they would do different things at some time soon in the future, something that never came to pass. The first case actually seems more like someone just doesn't understand how switch works, and performed a monkey-see-monkey-do to make the code work.

     

    Spot on! I think both cases is something like you suggest. There is no WTF in this code, the big WTF here is how little imagination people here have (except Volmaris), and obviously can't see the forest behind all the tree's.

  • (cs) in reply to Ken Nipper
    <!--StartFragment -->
    Ken Nipper:
     "Everyone thinks they have a sense of humor, but then they don't always." - <!--StartFragment --> Steve Martin

    "Some people think something is funny, then expect everyone else to think so too." - John Smallberries

  • (cs) in reply to Bytecodes is not codebytes
    Bytecodes is not codebytes:

    Volmarias:
    To be honest, this isn't all THAT bad. In both cases, one possible explaination is that at one time, different states did different things. However, they were changed such that they would all do the same thing, with the expectation that they would do different things at some time soon in the future, something that never came to pass. The first case actually seems more like someone just doesn't understand how switch works, and performed a monkey-see-monkey-do to make the code work.

     

    Spot on! I think both cases is something like you suggest. There is no WTF in this code, the big WTF here is how little imagination people here have (except Volmaris), and obviously can't see the forest behind all the tree's.

    I would agree that if functionality was changed due to differing requirements, then this would make at least some sense.  If that is the case though, then the WTF, in my opinion, is the lack of meaningful comments regarding the change. 

  • (cs) in reply to Volmarias

    Volmarias:
    In both cases, one possible explaination is that at one time, different states did different things. However, they were changed such that they would all do the same thing, with the expectation that they would do different things at some time soon in the future, something that never came to pass.

     

    Even if that is what happened, everyone reading the code is going to have to take the time to determine that the cases are the same.  When the requirement comes back, how long does it really take to add an if statement back in?

     

  • (cs) in reply to Ken Nipper
    Ken Nipper:
    Bytecodes is not codebytes:

    Volmarias:
    To be honest, this isn't all THAT bad. In both cases, one possible explaination is that at one time, different states did different things. However, they were changed such that they would all do the same thing, with the expectation that they would do different things at some time soon in the future, something that never came to pass. The first case actually seems more like someone just doesn't understand how switch works, and performed a monkey-see-monkey-do to make the code work.

     

    Spot on! I think both cases is something like you suggest. There is no WTF in this code, the big WTF here is how little imagination people here have (except Volmaris), and obviously can't see the forest behind all the tree's.

    I would agree that if functionality was changed due to differing requirements, then this would make at least some sense.  If that is the case though, then the WTF, in my opinion, is the lack of meaningful comments regarding the change. 

    I think the meaningful comments got lost someway down in the middle of "original code" and "code-snippet posted at thedailyWTF"

  • (cs)

    switch (state)
    {
        case Enumerations.State.Gollum:
          filthyHobbits.GetPreciousFrom()
          break;
       case Enumerations.State.Smeagol:
          filthyHobbits.GetPreciousFrom()
          break;
       default:
          filthyHobbits.GetPreciousFrom()
          break;
    }

  • (cs) in reply to Bytecodes is not codebytes
    Bytecodes is not codebytes:

    Volmarias:
    To be honest, this isn't all THAT bad. In both cases, one possible explaination is that at one time, different states did different things. However, they were changed such that they would all do the same thing, with the expectation that they would do different things at some time soon in the future, something that never came to pass. The first case actually seems more like someone just doesn't understand how switch works, and performed a monkey-see-monkey-do to make the code work.

    Spot on! I think both cases is something like you suggest. There is no WTF in this code, the big WTF here is how little imagination people here have (except Volmaris), and obviously can't see the forest behind all the tree's.


    Uh, huh.  So, I should add in lots of unecessary classes and extensive inheritence hierarchies just in case I might need them someday?

    Extra code is extra code that has to be read and udnerstood.  If the person wanted to be clear that all the cases were covered (s)he could've just put in all the cases before the two lines of code that were needed.  As it is, you have to do a whole bunch of reading to realize all the cases are the same, then you want to go strangle the original programmer.  It's a bad coding practice.

    It's even worse than having large swaths of commented out code (just in case you need to put it back in) when the code is under version control.  At least then most reasonable editors flag all the dead code as being a comment.

  • (cs)
    Jake Vinson:

    switch (state)
    {
    case Enumerations.PageState.Edit:
    //Load Page Data LoadPageData(); break;
    case Enumerations.PageState.Update:
    //Load Page Data LoadPageData(); break;
    case Enumerations.PageState.Add:
    //Load Page Data LoadPageData(); break;
    default:
    //Load Page Data LoadPageData(); }


    I have seen, on many occassions, people do this very same thing with exception handling.  For example:

    try{
        ...
    }catch(IOException ioe){
        log.debug("There was a IOException", ioe);
        throw ioe;
    }catch(SAXException saxe){

        log.debug("There was a SAXException", saxe);

        throw saxe;

    }catch(NumberFormatException nfe){

        log.debug("There was a NumberFormatException", nfe);

        throw saxe;

    }

  • Irrelevant (unregistered) in reply to OneFactor
    OneFactor:

    switch (state)
    {
        case Enumerations.State.Gollum:
          filthyHobbits.GetPreciousFrom()
          break;
       case Enumerations.State.Smeagol:
          filthyHobbits.GetPreciousFrom()
          break;
       default:
          filthyHobbits.GetPreciousFrom()
          break;
    }

    **applause**

    on another note, I have, on occasion, used a switch-case with lots of cases with the same code, and only the default doing anything different, purely 'cos it appealed to my sense of aestetics better than a mess of if(foo==a||foo==b||...). But then, that's what case fall-through is for, so that wouldn't even begin to explain the ^C^Ving going on here.

  • (cs) in reply to Irrelevant

    well, cack. bitten by not previewing 'cos the CAPCHA didn't work on preview. that's the last time i don't log in out of laziness. I still think it's distinctly daft that this forum software continues to be used here.

  • (cs) in reply to Bytecodes is not codebytes
    Bytecodes is not codebytes:

    Volmarias:
    To be honest, this isn't all THAT bad. In both cases, one possible explaination is that at one time, different states did different things. However, they were changed such that they would all do the same thing, with the expectation that they would do different things at some time soon in the future, something that never came to pass. The first case actually seems more like someone just doesn't understand how switch works, and performed a monkey-see-monkey-do to make the code work.

     

    Spot on! I think both cases is something like you suggest. There is no WTF in this code, the big WTF here is how little imagination people here have (except Volmaris), and obviously can't see the forest behind all the tree's.



    There's no doubt that there is a WTF here.  If you change the code, and that code change renders the surrounding code obsolete, remove the surrounding code.  In your scenario, the WTF is that they didn't remove the useless switch.  After all, we ARE supposed to be professionals.
  • ac (unregistered) in reply to Mung Kee
    Mung Kee:
    Jake Vinson:

    switch (state)
    {
    case Enumerations.PageState.Edit:
    //Load Page Data LoadPageData(); break;
    case Enumerations.PageState.Update:
    //Load Page Data LoadPageData(); break;
    case Enumerations.PageState.Add:
    //Load Page Data LoadPageData(); break;
    default:
    //Load Page Data LoadPageData(); }


    I have seen, on many occassions, people do this very same thing with exception handling.  For example:

    try{
        ...
    }catch(IOException ioe){
        log.debug("There was a IOException", ioe);
        throw ioe;
    }catch(SAXException saxe){

        log.debug("There was a SAXException", saxe);

        throw saxe;

    }catch(NumberFormatException nfe){

        log.debug("There was a NumberFormatException", nfe);

        throw saxe;

    }



    on a try { that throws NumberFormatException, SAXException, IOException, and FourthException, FifthException, SixthException, how do you catch the first three and let the last 3 propagate?

  • (cs) in reply to ac
    Anonymous:

    on a try { that throws NumberFormatException, SAXException, IOException, and FourthException, FifthException, SixthException, how do you catch the first three and let the last 3 propagate?


    Just don't re-throw the first 3.  (changed to log.error() too)

    try{
        ...
    }catch(IOException ioe){
        log.
    error("There was a IOException", ioe);
       
    //dont re-throw
    }catch(SAXException saxe){

        log.error("There was a SAXException", saxe);
        //dont re-throw

    }catch(NumberFormatException nfe){

        log.error("There was a NumberFormatException", nfe);
        //dont re-throw

    }catch(FourthException fourth){

        log.error("There was a NumberFormatException", nfe);
        throw
    fourth;  //DO re-throw

    }

  • DD (unregistered)

    For example:


    switch (state)
    {
    case Person.State.Happy:
    Drink();
    break;
    case Person.State.Unhappy:
    Drink();
    break;
    case Person.State.JusGotDumpedByGirlfriend:
    Drink();
    break;
    case
    Person.State.JusGotPayRise:
    Drink();
    break
    ;
    default:
    Drink();
    }

  • Mr Smith (unregistered) in reply to Mung Kee
    Mung Kee:

    try{
        ...
    }catch(IOException ioe){
        log.debug("There was a IOException", ioe);
        throw ioe;
    }catch(SAXException saxe){

        log.debug("There was a SAXException", saxe);

        throw saxe;

    }catch(NumberFormatException nfe){

        log.debug("There was a NumberFormatException", nfe);

        throw saxe;

    }


    At university, one of our lecturers taught us that this is the correct way to do it, so that you can clearly see which exceptions are being caught there.
    A certain tutor then marked down nearly the entire class for doing this on an assignment.

  • (cs) in reply to Mr Smith
    Anonymous:

    At university, one of our lecturers taught us that this is the correct way to do it, so that you can clearly see which exceptions are being caught there.
    A certain tutor then marked down nearly the entire class for doing this on an assignment.

    The same case can be made for the original posting ( switch (state) ), the make clear what  possible values "state" can be.  I say, who the hell cares if you going to do the same thing with it.
  • waffle cone (unregistered) in reply to Mung Kee

    Hmm.  Not removing useless code implies not being professional....

    <checks some="" code="" written="" by="" 'professionals="" for="" obvious="" lack="" of="" utility="">debug = false;
    ...
    if (debug) {
        ...
    }

    Bloody amateurs.

    Seriously, while that code is not exactly a shining example of intelligence others have proposed various reasonable origins.  Your time/effort is a resource when programming, and good programmers know to both optimize last and also to focus optimization on the important, often-executed code, which this may well not be.
    </checks>

    A truly good optimizing compiler should recognize the redundant code and common it up anyway.

  • (cs)
  • (cs) in reply to Mung Kee
    Mung Kee:
    Jakeypoo:

    switch (state)
    {
    case Enumerations.PageState.Edit:
    //Load Page Data LoadPageData(); break;
    case Enumerations.PageState.Update:
    //Load Page Data LoadPageData(); break;
    case Enumerations.PageState.Add:
    //Load Page Data LoadPageData(); break;
    default:
    //Load Page Data LoadPageData(); }


    I have seen, on many occassions, people do this very same thing with exception handling.  For example:

    try{
        ...
    }catch(IOException ioe){
        log.debug("There was a IOException", ioe);
        throw ioe;
    }catch(SAXException saxe){
        log.debug("There was a SAXException", saxe);
        throw saxe;
    }catch(NumberFormatException nfe){
        log.debug("There was a NumberFormatException", nfe);
        throw saxe;
    }

     

    About exception code snippet :

    1. If the WTF is doing "log.debug" instead of "log.error", I agree.

    2. Are you saying that it should be "catch (Exception ex)"?

    3. Individual exception handling is preffered (if you know what to do with it) although you may handle them in the same way.

    4. So this may make sense depending on the context. (i.e. This method may be in a middle layer, but the caller of this method may have to re-catch the same exceptions and dispaly a proper user friendly message in the UI layer).


    Original post :

    It is a horrible WTF.

    Just comment out the damn code when you are not using. And put some meaningful comments.

    If you can't do that, you are just a lazy programmer.

  • (cs)

    Well, I think it is sound axiomatic advise, let alone programming:

    switch (YouDontSucceed)
    {
       case At.First:
        Try();
        break;
       case At.Second:
        Try();
        break;
       case At.Second:
        Try();
        break;
      default:
        Try();
    }

  • (cs) in reply to kdd
    kdd:

    Original post :

    It is a horrible WTF.

    Just comment out the damn code when you are not using. And put some meaningful comments.

    If you can't do that, you are just a lazy programmer.



    Wrong answer on commenting out code.  If you don't need it, then take it out.  That's what source history tracking is for. 

    OTOH, agreed on the meaningful comments and this being a horrible WTF.
  • (cs) in reply to OMG

    DOH! - Make that:
    switch (YouDontSucceed)
    {
       case At.First:
        Try();
        break;
       case At.Second:
        Try();
        break;
       case At.Third:
        Try();
        break;
      default:
        Try();
    }

  • (cs) in reply to kdd
    kdd:

    About exception code snippet :

    1. If the WTF is doing "log.debug" instead of "log.error", I agree. 
    2. Are you saying that it should be "catch (Exception ex)"?
    3. Individual exception handling is preffered (if you know what to do with it) although you may handle them in the same way.
    4. So this may make sense depending on the context. (i.e. This method may be in a middle layer, but the caller of this method may have to re-catch the same exceptions and dispaly a proper user friendly message in the UI layer).

    Original post :

    It is a horrible WTF.

    Just comment out the damn code when you are not using. And put some meaningful comments.

    If you can't do that, you are just a lazy programmer.



    1.  Typo on my part, and corrected in a later post.
    2.  Yes
    3.  Not across application layers. 
    4.  It's generally not good design to throw multiple types of exceptions across layers.  If the layer is truly a 'black box', you certainly won't see it's public interface methods throw SAXExceptions.

    Original post :
    Lazy?  Belee dat
  • waffle cone (unregistered) in reply to Mr Smith
    Anonymous:
    Mung Kee:

    try{
        ...
    }catch(IOException ioe){
        log.debug("There was a IOException", ioe);
        throw ioe;
    }catch(SAXException saxe){

        log.debug("There was a SAXException", saxe);

        throw saxe;

    }catch(NumberFormatException nfe){

        log.debug("There was a NumberFormatException", nfe);

        throw saxe;

    }


    At university, one of our lecturers taught us that this is the correct way to do it, so that you can clearly see which exceptions are being caught there.
    A certain tutor then marked down nearly the entire class for doing this on an assignment.


    Well, if you applied that strategy everywhere then you indeed should have lost marks.  Suppose your call chain is e() calls f() calls g() calls h().  Now suppose h() throws those 3 exceptions, so in g() you put your above construct around the call.  Thus g() now throws those 3 exceptions, and so in f() you also put your above construct around the call to g().  Same thing for the call to f() in e().  Now your exception is logged 3 times when it should be logged only once.  


  • (cs) in reply to dubwai
    dubwai:

    Only one more day of this $hit Jake
  • Rabiator (unregistered) in reply to Ken Nipper
    Ken Nipper:
    Can anyone say "Copy and Paste"[:^)]

    and getting distracted before changing the pasted code to it's sightly different, final form.
    I had stuff like that happen to me during coding, but it usually showed up in testing. In this case, the developer did obviously not test his code.
  • (cs) in reply to waffle cone
    Anonymous:
    Anonymous:
    Mung Kee:

    try{
        ...
    }catch(IOException ioe){
        log.debug("There was a IOException", ioe);
        throw ioe;
    }catch(SAXException saxe){

        log.debug("There was a SAXException", saxe);

        throw saxe;

    }catch(NumberFormatException nfe){

        log.debug("There was a NumberFormatException", nfe);

        throw saxe;

    }


    At university, one of our lecturers taught us that this is the correct way to do it, so that you can clearly see which exceptions are being caught there.
    A certain tutor then marked down nearly the entire class for doing this on an assignment.


    Well, if you applied that strategy everywhere then you indeed should have lost marks.  Suppose your call chain is e() calls f() calls g() calls h().  Now suppose h() throws those 3 exceptions, so in g() you put your above construct around the call.  Thus g() now throws those 3 exceptions, and so in f() you also put your above construct around the call to g().  Same thing for the call to f() in e().  Now your exception is logged 3 times when it should be logged only once.  




    And you wasted the whole goddamn day writing try...catch blocks

  • (cs) in reply to OneFactor
    OneFactor:

    switch (state)
    {
        case Enumerations.State.Gollum:
          filthyHobbits.GetPreciousFrom()
          break;
       case Enumerations.State.Smeagol:
          filthyHobbits.GetPreciousFrom()
          break;
       default:
          filthyHobbits.GetPreciousFrom()
          break;
    }

    Perfect example.  Because at some point, the maintenance coder has to go in and change the Smeagol case to check if the Gollum state has been hit recently.  If not, do filthyHobbits.TryToPleaseTheSkinnyOne(), otherwise .GetPreciousFrom(), then later it has to go back to its original state.

    And for all the talk about professionals refactoring, some of us professionals consider the maintenance guy as well.  It's always a trade off.  Perfect elegant code is great right up until the point the requirements change (known to much of the world by it's common name, "delivery").

  • Sean Connery (unregistered) in reply to Volmarias
    Volmarias:
    To be honest, this isn't all THAT bad. In both cases, one possible explaination is that at one time, different states did different things. However, they were changed such that they would all do the same thing, with the expectation that they would do different things at some time soon in the future, something that never came to pass. The first case actually seems more like someone just doesn't understand how switch works, and performed a monkey-see-monkey-do to make the code work.


    If the language is C#, the WTF is that they used a swithc instead of an if. C# doesnt allow fall-through like C++.

    so C++ lets you do case A: case B: case C: // more readable than c# version
    but C# forces you to do case A: break; case B: break; case C: break;

    should use an if and say if(A || B || C) // more readable
  • (cs) in reply to Volmarias
    Volmarias:
    To be honest, this isn't all THAT bad. In both cases, one possible explaination is that at one time, different states did different things. However, they were changed such that they would all do the same thing, with the expectation that they would do different things at some time soon in the future, something that never came to pass. The first case actually seems more like someone just doesn't understand how switch works, and performed a monkey-see-monkey-do to make the code work.


    I can't accept this reasoning. How hard do one just comment out redundant part of the code?
    'If Request.Form("ml") = "Y" Then
    lblNavigation.Text = _
    ddlOperationList.Items.Item(ddlOperationList.SelectedIndex).Text
    'Else
    lblNavigation.Text = _
    ddlOperationList.Items.Item(ddlOperationList.SelectedIndex).Text
    'End If
    And that would a lot more eaily to follow in an editor.

    I often see this kind of branch redundant code when I maintaining someone else written code. It means that the original programmer is lacking proper programming skills, which their are hard to follow the logic, full of bugs and very difficult to maintain and extend. Then I have no choice but refactoring everything they wrote. This at least double the amount that what I should do.
  • (cs) in reply to Sean Connery

    Anonymous:
    If the language is C#, the WTF is that they used a swithc instead of an if. C# doesnt allow fall-through like C++.

    so C++ lets you do case A: case B: case C: // more readable than c# version
    but C# forces you to do case A: break; case B: break; case C: break;

    should use an if and say if(A || B || C) // more readable

    Are you sure C# doesn't have a better method? VB.NET uses the following:

    select case (something)
       case A, B, C: dostuff
       case D, E: dootherstuff
    end select

    I'm sure C# has a similar construct, ad after testing, it looks like it works exactly the same as the C++ variant:

    <FONT color=#0000ff size=2>

    switch</FONT><FONT size=2>(i)
    {
       </FONT><FONT color=#0000ff size=2>case</FONT><FONT size=2> 0:
       </FONT><FONT color=#0000ff size=2>case</FONT><FONT size=2> 1:
    </FONT><FONT color=#0000ff size=2>   case</FONT><FONT size=2> 2:
          i = 5;
          </FONT><FONT color=#0000ff size=2>break</FONT><FONT size=2>;
       </FONT><FONT color=#0000ff size=2>case</FONT><FONT size=2> 3:
          i = 6;
          </FONT><FONT color=#0000ff size=2>break</FONT><FONT size=2>;
    }</FONT>

    <FONT size=2>This results in i ==5 if i starts as 0, 1 or 2, and i==6 if i starts as 3.</FONT>

    <FONT size=2>Drak

    </FONT>
  • (cs) in reply to Omnifarious
    Omnifarious:
    Bytecodes is not codebytes:

    Volmarias:
    To be honest, this isn't all THAT bad. In both cases, one possible explaination is that at one time, different states did different things. However, they were changed such that they would all do the same thing, with the expectation that they would do different things at some time soon in the future, something that never came to pass. The first case actually seems more like someone just doesn't understand how switch works, and performed a monkey-see-monkey-do to make the code work.

    Spot on! I think both cases is something like you suggest. There is no WTF in this code, the big WTF here is how little imagination people here have (except Volmaris), and obviously can't see the forest behind all the tree's.


    Uh, huh.  So, I should add in lots of unecessary classes and extensive inheritence hierarchies just in case I might need them someday?

    Extra code is extra code that has to be read and udnerstood.  If the person wanted to be clear that all the cases were covered (s)he could've just put in all the cases before the two lines of code that were needed.  As it is, you have to do a whole bunch of reading to realize all the cases are the same, then you want to go strangle the original programmer.  It's a bad coding practice.

    It's even worse than having large swaths of commented out code (just in case you need to put it back in) when the code is under version control.  At least then most reasonable editors flag all the dead code as being a comment.

     

    Obviously my intention with my post was not that this isn't bad coding practise. But this code-snippet alone says so much why it is there. This stuff is quite often in my own code, but only temporary, I have a failsafe method of keeping track of where I do these temporary "test"s, so I always remove them, rewrite them, bring em back to original or whatever I do just as soon as I have completed my test. The tests could be anything, like testing a new function in all different application states and so on. It saves so lot of time that in just some seconds be able to do a test, in creating something like this. The clearly thing about this guy is that he lack practise in how to keep track of them, so he obviously missed to correct this test. If this code is in function that is never used and never will be used, we have no clue. Then he again saved time by not correcting it, but he should of had time to remove it from the project. But if he is only on this project, and no one ever will "take over" after him, I think this code-snippet illustrates "lack of time" a LONG LONG way.

Leave a comment on “1, 1, or 1, or if you really want, 1”

Log In or post as a guest

Replying to comment #40635:

« Return to Article