• Quite (unregistered)

    I feel OP's pain.

    I still find things like this, in code I'm given to review:

    if (thingToTest == 1) {
        doThing (1);
    }
    
    if (thingToTest == 2) {
        doThing (2);
    }
    
    if (thingToTest == 3) {
        doThing (3);
    }
    :
    :
    

    ...except I've formatted for neatness.

  • Primary Key (unregistered)

    I had once in a piece of VB-code

    If x = 0 Then y = 1 ElseIf x = 1 Then y = 2 ElseIf x = 2 Then y = 3

    etc up to 11.

    This was to select the right name of the month from an array. This code wasn't just once there but was copied all over the place. I cried a little on the inside.

  • Giulio (unregistered)

    I'm also quite scared by the default: break;, to be honest

  • Jules (unregistered)

    You mind if I have some of your tasty beverage to wash this value down with?

  • PedanticPete (unregistered)

    Being in a pedantic mood this morning I'll point out that the replace,ent code is not he same as what it replaced. It lacks the default case of doing nothing if the fi or doesn't match one of the predefined names. Thus a new and subtle bug has been introduced.

  • Vault_Dweller (unregistered) in reply to PedanticPete

    "Thus a new and subtle bug has been introduced."

    Or fixed.

  • Jens (unregistered) in reply to PedanticPete

    And pray that fieldName contains only the previously defined values.

  • JJ (unregistered)

    This reminds me of a piece of code I once had the misfortune to inherit. Software running a cash register. The transaction processing loop (which chewed through everything you'd done, resulting in something that was a human readable receipt) spanned twentyfour screens layed out just like this.

    I could sort of see how this happened ("We have loyalty coupons now? No worries, I'll make a case in the TPL for that"). It didn't make a fun piece of code to inherit, though.

  • ichbinkeinroboter (unregistered) in reply to PedanticPete

    Exactly what I was thinking! It needs a whitelist (or maybe more likely blacklist, context depending).

  • Ann Onymous (unregistered)

    I once had a (supposedly senior) coworker populate the hours in a day like;

    _dropdownList.Items.Clear(); _dropdownList.Items.Add("1"); _dropdownList.Items.Add("2"); ...[snip] _dropdownList.Items.Add("24");

  • BernieTheBernie (unregistered)

    case "<FRIST>": theCommands = theCommands.Replace("<FRIST>", WashTheValue(reader["FieldValue"].ToString(), reader["OrderingFieldID"].ToString(), reader["PriceFormat"].ToString())); break; Oh please... wash the value!!!

  • tbodt (unregistered)

    There's something the original code does that the replacement code doesn't: doing nothing when the template field name isn't in the list.

  • löchlein deluxe (unregistered)

    Eh, my crystal balls say the old code used to have strategically-places parseInt()s for some of the fields.

  • Anonymous (unregistered)

    Don't sprain your scrolling finger! You do know that clicking the mouse wheel on the page will enable auto-scroll, which allows you to scroll by moving the cursor up or down, right?

  • bobcat (unregistered)

    'CLEMATIS' made me giggle more than it should have.

  • jeepwran (unregistered) in reply to ichbinkeinroboter

    Or not because, this: "Even worse, as fields were added, someone would have to go in and update this block of code."

  • Axel (unregistered) in reply to Anonymous

    I'm having trouble finding the mouse wheel on my phone.

Leave a comment on “Cases, Cases, Cases”

Log In or post as a guest

Replying to comment #487395:

« Return to Article