Imagine you were browsing a C++ codebase and found a signature in a header file like this:
int max (int a, int b, int c, int d);
Now, what do you think this function does? Would your theories change if I told you that this was just dropped in the header for an otherwise unrelated class file that doesn't actually use the max
function?
Let's look at the implementation, supplied by Mariette.
int max (int a, int b, int c, int d)
{
if (c == d)
{
// Do nothing..
}
if (a >= b)
{
return a;
}
else
{
return b;
}
}
Now, I have a bit of a reputation of being a ternary hater, but I hate bad ternaries. Every time I write a max
function, I write it with a ternary. In that case, it's way more readable, and so while I shouldn't fault the closing if
statement in this function, it annoys me. But it's not the WTF anyway.
This max
function takes four parameters, but only actually uses two of them. The //Do nothing..
comment is in the code, and that first if
statement is there specifically because if it weren't, the compiler would throw warnings about unused parameters.
Those warnings are there for a reason. I suspect someone saw the warning, and contemplated fixing the function, but after seeing the wall of compiler errors generated by changing the function signature, chose this instead. Or maybe they even went so far as to change the behavior, to make it find the max of all four, only to discover that tests failed because there were methods which depended on it only checking the first two parameters.
I'm joking. I assume there weren't any tests. But it did probably crash when someone changed the behavior. Fortunately, no one had used the method expecting it to use all four parameters. Yet.
Mariette confirmed that attempts to fix the function broke many things in the application, so she did the only thing she could do: moved the function into the appropriate implementation files and surrounded it with comments describing its unusual behavior.