The Daily WTF: Curious Perversions in Information Technology

2010-09-22 Reply Admin

People on this site seem to bring up "commenting your code" as a solution, but in general if the code didn't make sense after reading it, reading a code comment generally doesn't make it all "clear."

Anyway, comments in code usually lie. Learn how to use a debugger.

2010-09-22 Reply Admin

adunn:
Because the formula should look familiar to anyone who has ever generated any kind of graphical grid (likely to happen in GUIs). The code should at least be using meaningfully named variables, both for readability and for the likeliness that these number will be reused to line other things up. I don't think the formula needs a comment in the context of a GUI.
rect.X = offsetX + ((i - 1) % numColumns) * cellWidth;
rect.Y = offsetY + ((int)((i - 1) / numColumns)) * cellHeight;
Make more sense?

Totally fixed! Thanks.

2010-09-22 Reply Admin

This code isn't really that hard to follow, seems to be setting up a general layout.... the wtf might be the submitter.

2010-09-22 Reply Admin

Indeed...

7 is not such a big number. And this would be much clearer...

Rectangle[] rect = new Rectangle[7]; //header rect [0] = new Rectangle(216, 12, 110, 60); //1st row rect [1] = new Rectangle( 4, 75, 105, 60); rect [2] = new Rectangle(110, 75, 105, 60); rect [3] = new Rectangle(216, 75, 105, 60); //2nd row rect [4] = new Rectangle( 4, 135, 105, 60); rect [5] = new Rectangle(110, 135, 105, 60); rect [6] = new Rectangle(216, 135, 105, 60);

for (int i = 0; i<7; i++){ //whatever common code }

2010-09-22 Reply Admin

nasch:
For people defending this, you really wouldn't mind writing this code and leaving it without any comments?
rect.X = 4+((i-1) % 3)*106;
rect.Y = 75+((int)((i-1)/3))*60 ;
Six (possibly seven, if you count the 1) undocumented literal numbers in two lines of code? I don't care if it's a UI or a SQL statement or what, that is just terrible.

I suspect it was coded just to recognize those who can see the forest (overall pattern that's created by the calulated coordinates) from the trees (magic number, ohmygod, call the coding style police).

Alternatively, it's a poor attempt at job security. But perhaps more effort was spent on that in the rest of the code (shudder).

Anyway, this one is really not so terrible, and it's easy to imagine a reasonable context around the given piece of code.

Oh, and naturally having magic numbers like that is perfectly reasonably if it's an embedded device with fixed size screen, but I bet somebody has already pointed that out... ;-)

2010-09-22 Reply Admin

Guardian Bob:
There's no excuse for not documenting it, which is why I said it needs to be better documented (okay I was a little less forceful than that in my language).
The code isn't too insane, I mean I ran across this last week:
int foo(int bar)
{
  if(some_long_condition(bar))
    return 0;
  return 0;
}
The submitted CodeSOD doesn't make me question my own moral and ethical liability if I let the author touch a compiler again. I mean training some to document better isn't as hard as training someone to think about what their doing.

Did you comment it out and the whole thing break? Sometimes people use an "if" statement for in-order, shorthand evaluation of a bunch of statements. The last return statement is just because the function was declared to return something, and the compiler would complain otherwise. Hopefully it comes with a comment to such effect, though.

frits · 2010-09-22 Reply Admin

Long:
Guardian Bob:
There's no excuse for not documenting it, which is why I said it needs to be better documented (okay I was a little less forceful than that in my language).
The code isn't too insane, I mean I ran across this last week:
int foo(int bar)
{
  if(some_long_condition(bar))
    return 0;
  return 0;
}
The submitted CodeSOD doesn't make me question my own moral and ethical liability if I let the author touch a compiler again. I mean training some to document better isn't as hard as training someone to think about what their doing.
Did you comment it out and the whole thing break? Sometimes people use an "if" statement for in-order, shorthand evaluation of a bunch of statements. The last return statement is just because the function was declared to return something, and the compiler would complain otherwise. Hopefully it comes with a comment to such effect, though.

?

2010-09-22 Reply Admin

Dear Emil,

In case you can’t tell, this is a grown-up place. The fact that you don't know how to program simple 2D graphics clearly shows that you’re too young and too stupid to be using computers.

Go away and grow up.

Sincerely, Bert Glanstron

2010-09-22 Reply Admin

History Teacher:
I suspect it was coded just to recognize those who can see the forest (overall pattern that's created by the calulated coordinates) from the trees (magic number, ohmygod, call the coding style police).

Ah yes, that old argument...

Let's take the current example. If I'm, say, looking for a section where it sets a color for displaying some doodad or another (further down in the code), I'd be stuck looking at every line to figure out WTF it was doing. If there were some comments and meaningful variable names (and #define'd quantities), I could visually scan (or search/grep) for words of importance. Huge difference in readability and the resulting productivity.

I shouldn't have to comb through every single line to find a section of interest. Proper naming of variables and simple comments go a long way to aid in code overview; it's the difference between actually having to read all of the code in order to find one specific function and being able to judge flow and structure at a glance (more or less). Code should be written in a way that actively promotes its understanding and readability. Simply lacking obscurity isn't enough.

History Teacher:
Oh, and naturally having magic numbers like that is perfectly reasonably if it's an embedded device with fixed size screen, but I bet somebody has already pointed that out... ;-)

Until the software needs to be updated for a new version of the device, which is mostly the same but has a display with (even slightly) different dimensions. Instead of simply updating your #define'd values, you're stuck trying to replace every occurrence of an integral constant - and you'll likely have to check each one to see if it was really being used for a display size instead of some other literal value.

2010-09-22 Reply Admin

DrJDX:
Matt Westwood:
[OK, so it has to be relevant to the article. I understand...]
Ah, magic number 7. I personally don't like magic numbers in my code but it is interesting to note that 7 is the number of cocks I've had in my mouth today since 7:00am. I don't know if this is some sort of sign or just a completely random coincidence but I must say that it's given me a boost and I'm very excited to think that my cock-munching will continue as soon as I've finished my break.

So then, back to it...
Hey Pal, we don't use that kinda language on this board. 7:00 AM is simply inappropriate subject matter, and I'll have to ask you to refrain from mentioning it in further discussion.

+1

Isn't that ironic?

2010-09-22 Reply Admin

This is GUI programming. By default GUI programming is ugly.

GUI programming with magic numbers is a sin, but not very great because GUI programming is ugly no matter what way you do it.

2010-09-22 Reply Admin

It's obvious he's just making a group of controls (buttons?) 3 columns wide, with 4 and 75 being the margins and 105 and 60 being the width and height.

Eternal Density · 2010-09-23 Reply Admin

frits:
Seven deadly sins Seven ways to win Seven holy paths to hell, and your trip begins
Seven downward slopes Seven bloodied hopes Seven are your burning fires Seven your desires

Reminds me of Dwarf Fortress, except that doesn't have any ways to win.

C4I_Officer · 2010-09-23 Reply Admin

Even if there was a constant settable at runtime through a modified classloader loading its configuration from an XML file retrieved from a database with three-tier authentication by using a stored procedure generated by an external PERL script, along with full internationalization of the constant's name in 160 different languages, still someone here would not be pleased and consider it a WTF that's it's not as configurable as he's like.

2010-09-23 Reply Admin

nasch:
adunn:
Because the formula should look familiar to anyone who has ever generated any kind of graphical grid (likely to happen in GUIs). The code should at least be using meaningfully named variables, both for readability and for the likeliness that these number will be reused to line other things up. I don't think the formula needs a comment in the context of a GUI.
rect.X = offsetX + ((i - 1) % numColumns) * cellWidth;
rect.Y = offsetY + ((int)((i - 1) / numColumns)) * cellHeight;
Make more sense?
Totally fixed! Thanks.

What was 'i' again?

Can we call it loopIterator instead?

<disclaimer>No, this is not a serious post </disclaimer>

2010-09-23 Reply Admin

Ares:
This is GUI programming. By default GUI programming is ugly.
GUI programming with magic numbers is a sin, but not very great because GUI programming is ugly no matter what way you do it.

GUI programiing is ugley, because GUI programmers are generally shyte, hence the need for all these drag'n'drop screen designers, which also generate ugly code, thus appeaqring that said shyte programmers haev put the same effort in that they used to, even though they haven't...

interestingly (no chip on my shoulder) any issues are never in the GUI, despite the ugly code (which I suppose makes the case that readability isn't important)

2010-09-23 Reply Admin

Everyone should know by now that THREE is the magic number.

2010-09-23 Reply Admin

it's bad because there is no separation of concerns. doesn't matter whether you can make some sense of it or not.

2010-09-23 Reply Admin

Gondolf:
TRWTF is naming. The first line looks like he's instantiating an interface...
I'm in ur base, Selecing your variablelst.

This is what code looks like when you let francophones write it. I remember one junior spending all day with visual studio, trying to create a method with accents in the name.

2010-09-23 Reply Admin

TRWTF is that list is spelled lst. I never get why people think that omitting one character is helpful in any way.

frits · 2010-09-23 Reply Admin

Eternal Density:
frits:
Seven deadly sins Seven ways to win Seven holy paths to hell, and your trip begins
Seven downward slopes Seven bloodied hopes Seven are your burning fires Seven your desires
Reminds me of Dwarf Fortress, except that doesn't have any ways to win.

It's Iron Maiden.

2010-09-23 Reply Admin

No, really? Never would have guessed.

2010-09-23 Reply Admin

I'll ditto those who say this is not that big a deal.

I hate magic numbers as much as anybody. But what is the programmer supposed to do in a case like this. Sure, you could write:

final static int CELL_WIDTH=105;
...
rect.width=CELL_WIDTH;

But is it really that cryptic to write "width=105"? Above this he spaces out boxes by an amount equal to width plus 1. Again, okay, you could write "x=LEFT_MARGIN+(i%3)*(CELL_WIDTH+1)". But would that really gain that much?

We might wonder why he chose 105 as the cell width and not some other number. But odds are this was a number that just worked out empirically.

2010-09-23 Reply Admin

Jongles:
nasch:
adunn:
Because the formula should look familiar to anyone who has ever generated any kind of graphical grid (likely to happen in GUIs). The code should at least be using meaningfully named variables, both for readability and for the likeliness that these number will be reused to line other things up. I don't think the formula needs a comment in the context of a GUI.
rect.X = offsetX + ((i - 1) % numColumns) * cellWidth;
rect.Y = offsetY + ((int)((i - 1) / numColumns)) * cellHeight;
Make more sense?
Totally fixed! Thanks.
What was 'i' again?

Can we call it loopIterator instead?

<disclaimer>No, this is not a serious post </disclaimer>

That's better, but incomplete. You forgot to mention that rect.X and rect.Y should be onScreenRectangle.pixelOffsetFromLeftEdge and onScreenRectangle.pixelOffsetFromTopEdge. Friggen duh! How do you get any work done when you spend so little time on these things?!

2010-09-24 Reply Admin

Guardian Bob:
The code isn't too insane, I mean I ran across this last week:
int foo(int bar)
{
  if(some_long_condition(bar))
    return 0;
  return 0;
}

TRWTF is not having a single exit point?

I've seen something similar in production code at my previous job:

if ($some_condition) {
  do_this();
} else {
  do_this();
}

That was written by a master in CS to execute either a function or the same function based on the booelan value of an undeclared varialbe. Since I'm a humble bachelor, his way of doing things was always better according to management.

To save you all the trouble, possible TRWTFs are: 1) using undeclared variables, 2) PHP allowing this, 3) PHP, 4) extraneous statements/unnecessary branching, 5) placeholder for a real check slipping into production (without any comment), 6) management. Definitely 6!

2010-09-24 Reply Admin

Testing if the array-element is null and then trying to call a function on thtat object doesnt make much sense...

if array(i).nil? array(i).do_something end

2010-09-25 Reply Admin

The real WTF is the incompetence of the readers here, who can't understand this glaringly obvious code.

Should be obvious that he or she is creating seven rectangles, the first one (index 0) at the coordinates of the "new" statement, the remaining six in two rows of three columns. It's just coordinates on the screen where he or she wants the rectangles to appear. No need to document that.

2010-09-26 Reply Admin

Really. NOOB. Moonchild ...

2010-09-26 Reply Admin

if ($condition)
{
    do_this();
}else{
    do_this();
}

isn't necessarily a WTF.

It could be that the requirements have changed and do_this() now needs to be executed all the time, and the developer left the $condition in, just in case it needs to be changed back at a later stage. It's possible that this could be foresight on the original/maintenance programmer's behalf.

Or, maybe the do_this_2() function was broken/breaking other things and not a mission critical function (GUI update or font formatting, or something) and the dev used a temporary replacement to get the project back online.

2010-09-27 Reply Admin

Anon y Mouse:
if ($condition)
{
    do_this();
}else{
    do_this();
}
isn't necessarily a WTF.

It could be that the requirements have changed and do_this() now needs to be executed all the time, and the developer left the $condition in, just in case it needs to be changed back at a later stage. It's possible that this could be foresight on the original/maintenance programmer's behalf.

Or, maybe the do_this_2() function was broken/breaking other things and not a mission critical function (GUI update or font formatting, or something) and the dev used a temporary replacement to get the project back online.

Sounds like a WTF to me. He couldn't be bothered to drag his mouse and hit the delete key?

Silly code like that should stay on your dev box if you are "just debugging", lest it be privy to the other developers who have a solemn duty to ridicule your poor choice of programming paradigms.

capthca: acsi - Standard for printing on a dyslexic machine

Magic Number 7

Leave a comment on “Magic Number 7”