Amusing or annoying code

[allistar]

Anyone who has been a developer in a large (and not so large) system long enough stumbles across what I call "WTF code". In some cases a sense of humility kicks in when you realise you wrote it years ago but now know better.

Here's a couple I can think of:

Playing with Booleans:

Code: Select all

if (someBoolean = true) then //do something elseif (someBoolean = false) then //do something else endif;

Code: Select all

if (someBoolean) then boolean1 := true; boolean2 := true; else boolean1 := false; boolean2 := false; endif;

Eek!

What kind of annoying, amusing or pointless code have you seen in production systems that fit this category?

[BeeJay]

One example of pointless code that I see frequently that irritates me no end:

Code: Select all

if someCollection.size > 0 then foreach obj in collection ... endforeach; endif;

Cheers,
BeeJay.

[BeeJay]

Try to work out what the developer was intending this code to achieve:

Code: Select all

if not client.isSpecialClient() = false then

[allistar]

A particular coding style I don't like it when indentation gets too deep, especially for something like error checking:

Code: Select all

hasError := doSomething(); if (not hasError) then hasError := doSomethingElse(); if (not hasError) then hasError := doSomethingYetAgain(); if (not hasError) then //you get the picture endif; endif; endif;

I have seen systems do that where there are as many as a dozen nested if statements.

[Dr Danyo]

Try to work out what the developer was intending this code to achieve:

Code: Select all

if not client.isSpecialClient() = false then

Nice.

Perhaps the simplest refactor there is, but what a different it makes to understanding.

http://www.refactoring.com/catalog/remo ... ative.html

Dr Danyo.

[ghosttie]

The one I hate the most is using untyped data - for example passing an Object around when it should really be a more specific type - inviting runtime errors at a later date.

The ultimate manifestation of this is methods like the following, which will break if any of the contents of the object arrays aren't the right class, but also unless all of the arrays are the exact same size:

Code: Select all

methodName(param1 : ObjectArray; param2 : ObjectArray; param3 : ObjectArray); vars iPos : Integer; begin foreach iPos in 1 to param1.size do param1[iPos].MyClass1.doSomething(param2[iPos].MyClass2, param3[iPos].MyClass3); endforeach; end;

This "pattern" seems to be most common with developers that originally learned programming in a non-OO language and never adjusted.

[andy-b]

Not sure if this qualifies as amusing or annoying? I found this on a method that checks to see if a user has security access to an item on the menu (have removed the processing code, but you get the idea)

Code: Select all

doesUserHaveAccessToNavigationItem( pNavigatableItemName : String ) : Boolean begin return true; [ Continue doing proper validation checking here and return value as appropriate... ] end;

So this is a pretty slutty method that will let anyone do anything. The method was last modified in 2001
Actually not that bad as is seems...I have found out this is a secondary security measure that is not usually relied upon.

[JohnP]

How about a line of dodgy code, with a comment saying "// TEMP!!", and dated 12 years ago?

[ghosttie]

The one I'm guilty of is

Code: Select all

// ugly hack (TM)

[Stokes]

we have got a global constant called "Min_Positive" which is defined as a Real equal to 0.

So instead of just putting in 0 for example

if collection.size > 0 then

we have a developer who puts in

if collection.size > Min_Positive then

he tells me this is the proper way of developing in Jade.... I find that hard to believe