Code I Found

Its time for a new category here on my blog... I've been working through a good bit of older code at work and trying to update it to something like modern developer standards. Part of the fun of this has been finding weird, pointless language constructs. Check out this C# example:

if (IsEditable(ID))
    EnableEditButtons(true);
else
    EnableEditButtons(false);

Neat! This developer was able to use 4 lines of code to do one line of code:

EnableEditButtons(IsEditable(ID));

Do you have any good examples of code that just shouldn't have been?

Update!

The EnableEditButton() function from the last example is worth a look too...

protected void EnableEditButtons(bool Value)
{
    if (Value == false)
    {
        buttonAdd.Disabled = true;
        buttonDelete.Disabled = true;
    }
    else
    {
        buttonAdd.Disabled = false;
        buttonDelete.Disabled = false;
    }
}

Could be wrapped up as:

protected void EnableEditButtons(bool Value)
{
    buttonAdd.Disabled = buttonDelete.Disabled = !Value;
}

 

Comments

Russ S.'s Gravatar Your examples are more efficient, but whenever you combine multiple operations in one line of code, you lose some readability. You could rewrite a whole application into one uber-efficient line of code, but it probably wouldn't be as cool as you think.

Also, if someday you needed to perform additional tasks depending on the value passed to enableEditButtons(), you'd have to revert to the original if/then statement anyways.

It's tempting to fix every little thing your predecessor did wrong, but it sure wastes a lot of time. I'm not trying to promote "bad code" here, but clients honestly don't care what the code looks like, as long as it works.

I try to follow these wise words when coding:
"If it ain't broke, don't fix it" and "Keep It Simple, Stupid"

Now the question becomes: what will you do with all the time I've just saved you?
Robert G's Gravatar Even thought I despise old code and poor coding habits I have to agree with Russ. Every line of code you change should be tested, even something as simple as your first example with the IsEditable(ID).
Jon Hartmann's Gravatar Russ, you're absolutely right that I you can save time and energy by not worry about things like this, and I usually don't. The real gains are when you see that that same developer had loaded 68,000 records into the view state to power a drop down that no one used... took the page from 5+ minute load time to 1-2 seconds. These examples (and the rest of what I put in the "Code I Found" category) are here just because I found them amusing, or they expend a lot of lines of code doing something very small. Sometimes you have to make updates like these just to get the code <i>into</i> a state that can be easily read.

In the end I know that these changes don't save time, just as you say. Things to think about. Thanks!
christine's Gravatar hi russ s.!
You have find out what i am also realized while looking code, anyway nice work...
Comments are not allowed for this entry.
Jon Hartmann, July 2011

I'm Jon Hartmann and I'm a Javascript fanatic, UX/UI evangelist and former ColdFusion master. I blog about mysterious error messages, user interface design questions, and all things baffling and irksome about programming for the web.

Learn more about me on LinkedIn.