The Daily WTF: Curious Perversions in Information Technology

2010-02-02 Reply Admin

Zapp Brannigan:
Like in an embedded system.

Wouldn't happen to be one without a filesystem, would it?

2010-02-02 Reply Admin

Snip-snip-snip...

frits:
Nice run-on sentence...

Thanks.

frits:
I notice you didn't actually address my reasons for suggesting a switch or other procedural construct is the wrong strategy.

This is because you haven't given any. You've made a whole lot of blanket assumptions about the context of the code, and stated that a switch (or if-elseif) statement is wrong, and that polymorphism is the "right" solution. For all you know this is part of the function or method that is always used to set these particular variables. There is no particular reason to assume that a polymorphic approach would be superior, nevertheless you're sure it is...

frits:
I learned OO on the job, not college. Did you go to college? I suspect you wouldn't know how to go about doing this using Polymorphism. You write a base class that implements most of the logic. Then you create simple classes that customize or extend the base class. It's really simple.

Yes I did. That's where I learned functional, procedural, modular and OO programming techniques (various styles), and a little about machine-code programming. My minor was Logic. After that I worked commercially for 13 years, 10 in a senior position. I enjoy kayaking, target shooting and long walks on the beach. But enough of the resume.

The approach you're suggesting is more complicated than a dozen-line switch statement. You're writing at least 2 more class definitions, probably more than that from your remarks. From a maintenance perspective (that's what the guy who comes after the designer does, and the situation we find ourselves in looking at a piece of pre-existing code), less is more. A mature project will have many thousands of class definitions (hopefully in a well organised framework) which need to be sifted through in order to make modifications or add new features. Object instantiation also means (implicit) memory allocation, which can be a problem if you're replacing simple program structures with objects (I was fixing just such a problem last week, out of memory errors, but I digress). How do you justify this additional layer of complexity?

Bear in mind that simplicity is one of those higher criteria that should always come before rigid adherence to a single programming style or paradigm.

frits:
Feeling superior and being superior are not always the same thing.

Indeed. This is why I find your so amusing.

2010-02-02 Reply Admin

If you want to optimize for the fewest total number of operations (including conditional branches) without using a lookup table, and limiting division to that which can be optimized into bit shifts, how about this:

if ((unsigned)(mainType-7) < (mainType&1)*5)
    subType = ((mainType+3)*mainType)/16;

I believe it beats RandomNameOfHiding's solution, which takes a similar approach...

2010-02-02 Reply Admin

frits:
blah blah blah GIGANTIC!!!

Laughing Jack:
blah blah blah ENORMOUS!!!

Quick, everyone, gather round! frits and Laughing Jack are about to whip out their Senior Programming Penii!!! (Penises? sp? gr?)

tl;dr: A shotgun might fix this. Assuming anyone not frozen in terror by meh code and stylistic choice is still capable of making a design decision and/or maintenance assumption about what type of Apocalypse we're really facing, here... (is it Zombie? Possibly Swamped.)

gtfo||stfu pls; kthxbai.

2010-02-02 Reply Admin

Laughing Jack:

frits:
Feeling superior and being superior are not always the same thing.

Indeed. This is why I find your so amusing.

I think you accidentally your smug there.

2010-02-02 Reply Admin

frits:
Any procedural solution for what this code is attempting would be smelly. However, I must say this is probably one of the ugliest ways to do it.

You're free to implement whatever your signer will pay you for, but i'm still not convinced that XMLing this up is the proper solution here, ol' chap.

2010-02-02 Reply Admin

Alan:

types = {
    7: 4,
    9: 6,
    11: 9,
}
if baseType not in types:
return None
return types[baseType]

Python FTW!

return {7: 4,
        9: 6,
        11: 9
       }.get(baseType, None)

Getting rid of the unnecessary conditional... FTW

2010-02-02 Reply Admin

quisling:
Laughing Jack:

frits:
Feeling superior and being superior are not always the same thing.

Indeed. This is why I find your so amusing.
I think you accidentally your smug there.

Terrible shame! Right at the end of a saving throw against DifferentThink, too... tsk, tsk.

2010-02-02 Reply Admin

Deadcode:
If you want to optimize for the fewest total number of operations (including conditional branches) without using a lookup table, and limiting division to that which can be optimized into bit shifts, how about this:
if ((unsigned)(mainType-7) < (mainType&1)*5)
    subType = ((mainType+3)*mainType)/16;

And here's one with even fewer operations and no multiplication:

if (mainType & (mainType-7 < 5u))
    subType = mainType + ((mainType-18)>>2);

2010-02-02 Reply Admin

frits:
You write a base class that implements most of the logic. Then you create simple classes that customize or extend the base class. It's really simple.

No no no. Base class? Inheritance? No. You need interfaces! That way you can dependency-inject the strategy class from your XML file.

public interface MainTypeStrategy {
    public int getSubtype(int mainType);
}

public class MainTypeSevenStrategy implements MainTypeStrategy {
    public int getSubtype(int mainType) {
        return 4;
    }
}

public class MainTypeNineStrategy implements MainTypeStrategy {
    public int getSubtype(int mainType) {
        return 6;
    }
}

public class MainTypeElevenStrategy implements MainTypeStrategy {
    public int getSubtype(int mainType) {
        return 9;
    }
}


public class SomeClass {
    private MainTypeStrategy mainTypeStrategy;

    public int convertMainTypeToSubType(int mainType) {
        return mainTypeStrategy.getSubtype(mainType);
    }

    public setMainTypeStrategy(MainTypeStrategy mainTypeStrategy) {
        this.mainTypeStrategy = mainTypeStrategy;
    }
}

Wow! That was so easy. And maintainable. No one will ever be confused as to why I wrote so much code instead of an if-else block! Adding a new class in the future will be sooo much easier than adding an extra else-if line.

But seriously, patterns are good.... just not here.

2010-02-02 Reply Admin

class MainType { bool operator== (int x) { return true ; } } mainType;

Welcome to the alternate universe ;-)

2010-02-02 Reply Admin

Sorry, but all these mathematical operations are conceptually wrong. Both variables (mainType and subType) contain the value of a "type", they do not contain a quantity; so it is conceptually wrong to perform mathematical operations on them.

The requirements are to set a certain value to subType according to certain rule based on the values of mainType. You are abusing the fact that the current values of those codes match your needs to perform an unneeded 'optimization' that also obscures the code.

A simple switch statements seems much wiser to me.

2010-02-02 Reply Admin

Ok, in this particular case a switch/case would have been clearly preferable. But generally a construct like

do {
  if (!precondition_1(data)) 
    break;
  preprocess(data)
  if (!precondition_2(data)) 
    break;
  ...
  mainAction(data);

} while(false);

for example is a valid construct and imo more readable than several nested if clauses or "goto out;". And once you are used to this pattern it is not that far fetched to use it the way it is used in the article. At least I think my code is a valid example for using a while loop without the intention to run it more than once.

2010-02-02 Reply Admin

enim:
Sorry, but all these mathematical operations are conceptually wrong. Both variables (mainType and subType) contain the value of a "type", they do not contain a quantity; so it is conceptually wrong to perform mathematical operations on them.
The requirements are to set a certain value to subType according to certain rule based on the values of mainType. You are abusing the fact that the current values of those codes match your needs to perform an unneeded 'optimization' that also obscures the code.

A simple switch statements seems much wiser to me.

Oh, you're right of course. But it's fun to make these kinds of optimizations. If you insist on maintability:

#ifdef ENABLE_EXTREME_OPTIMIZATIONS
if (mainType & (mainType-7 < 5u))
  subType = mainType + ((mainType-18)>>2);
#else
switch (mainType)
{
  case  7: subType = 4; break;
  case  9: subType = 6; break;
  case 11: subType = 9; break;
}
#endif

Cbuttius · 2010-02-02 Reply Admin

Yeah we all know you can use switch and should probably use enums or defined constants instead of magic numbers.

The question here is what are mainType and subType and whether a switch / if-else block should be used at all.

That would depend a lot on the rest of the code and where else this variable is checked, but the fact it is called "type" suggests it defines a type of some description and should probably use polymorphism, with possibly something along the lines of...

virtual enum ESubType getSubType() const;

and of course the subtype itself might also be a polymorphic class.

You might want each class to have a unique identifying numeric code if you are going to serialize it or persist it in some way and then will have to construct it back again. In that case you should register a factory against the enumeration to construct the appropriate class.

Once again too many comments on here pick on the "obvious" and fail to see the overall picture.

2010-02-02 Reply Admin

@Deadcode: The switch statement is still not maintainable. Obviously the code avoid magic numbers to improve readability.

#define FIRST_CASE 7
#define SECOUND_CASE 9
#define THIRD_CASE 11
#define MAPCODE_X 4
#define MAPCODE_6 6
#define MAPCODE_9 9

#define STOP ;break;

switch(mainType) {
  case FIRST_CASE: subType =MAPCODE_4 STOP
  case SECOUND_CASE: subType =MAPCODE_6 STOP
  case THIRD_CASE: subType =MAPCODE_9 STOP
  default: subType = subType STOP
}

2010-02-02 Reply Admin

sjakie:
Why can you mortals not accept this highly optimized piece of code? Everybody knows switch statements are a CPU killer, even worse than an if...else construct. This sub contractor really knew what he was doing, although I expect he would have rather written it in a proper language like assembly to begin with.

Obvious troll is obvious.

I will now commit harakiri for having propagated the meme....

Cbuttius · 2010-02-02 Reply Admin

Thorsten M.:

@Deadcode: The switch statement is still not maintainable. Obviously the code avoid magic numbers to improve readability.

#define FIRST_CASE 7
#define SECOUND_CASE 9
#define THIRD_CASE 11
#define MAPCODE_X 4
#define MAPCODE_6 6
#define MAPCODE_9 9

#define STOP ;break;

switch(mainType) {
  case FIRST_CASE: subType =MAPCODE_4 STOP
  case SECOUND_CASE: subType =MAPCODE_6 STOP
  case THIRD_CASE: subType =MAPCODE_9 STOP
  default: subType = subType STOP
}

enum is better than #define, but either way it is not extensible. If you add another case you must edit the code.

Using polymorphism you can add new implementations without modifying the existing code.

It is likely also that this number is used in more than one place, in which case if you add in another case you have to modify the code in several places.

switch has its purpose: where an object changes its state during its lifetime. Also commonly where there are two special cases, 0 and 1, and everything else follows the "default" path (and it is not related to the type of an object but something like the size of a returned data-set).

2010-02-02 Reply Admin

Short and beautiful.

s = ((m == 7) ? 4 : (m == 9) ? 6 : (m == 11) ? 9 : 0);

2010-02-02 Reply Admin

I've always used that in all my programs: (hello world) void main(){ while(true){ cout << "Hello World!" << endl; break; } }

The advantage is that your code only executes in dimension where "true" is true. You wont believe the things your algorithms can get up to when logic no longer works like here.

Cbuttius · 2010-02-02 Reply Admin

Chrulle:
I've always used that in all my programs: (hello world) void main(){ while(true){ cout << "Hello World!" << endl; break; } }
The advantage is that your code only executes in dimension where "true" is true. You wont believe the things your algorithms can get up to when logic no longer works like here.

In C++ main must return int not void

2010-02-02 Reply Admin

@Cbuttius Ok, obviously my proposal with the defines was just fooling around (the deliberately bad choice of names for the define might give a clue...).

If I wanted a clean and well readable C implementation, I'd probalbly do it this way:

#define INVALID -1 // This value must not be used as mainType

typedef struct  {
	int mainType;
	int subType;
} mapTableElement;

mapTableElement mapTable[] = {
	{ 7, 4 },
	{ 9, 6 },
	{ 11, 9 },
	/* Add new table elements here */
	{ INVALID, 0 } // This must remain last element
};

...
while( INVALID != mapTable[index].mainType   ) {
	if ( mainType != mapTable[index].mainType ) {
	  subType = mapTable[index].subType;
		break;
	}
	index++;
}

or shorter, easier maintainable (separate code from data), more runtime efficient and maybe even more secure (duplicate mainType entries are discovered as compile time) but probably makes the toenails of most OO developers curl:

source.c:

#define mapElement(a,b) case a: subType = b; break;
	switch( mainType ) {
#include "map.m"
		default:
			break;
	}

map.m:

mapElement(7,4)
mapElement(9,6)
mapElement(11,9)

Cheers, Throsten

Cbuttius · 2010-02-02 Reply Admin

If I were doing it in C (rather than C++), and the main types were close enough to 0, I would not do it your way but would use an array something like:

enum MapTypes { /*insert them here with assigns if necessary, in ascending order */ MAP_TYPE_SIZE }; // MAP_TYPE_SIZE is one more than the last element

int SubTypes[ MAP_TYPE_SIZE ]; // or type would be an enum rather than int

// put initialisation code here

If it is C++ you can improve it by having an object with a constructor that constructs the table for you, thus you don't need to remember to call the initialisation function (although with C++ you would probably use polymorphism because it's clean enough, while with C passing function pointers about is very messy).

The big advantage though is that you get constant time lookup.

2010-02-02 Reply Admin

@Cbuttius

Why use an enum here? How would you name the mambers? map_type_0, map_type_1, ... ? If they are close to zero I'd use a table rightaway:

int subTypesLookupTable[] = {
/*0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 */
  0, 0, 0, 0, 0, 0, 0, 4, 0, 6,  0,  9 };

No explicit initialization required, just check

0 <= mainType < sizeof(subTypesLookupTable) / sizeof(int)

(sizeof is executed at compile time.) But the example looks like the tale is not that populated, so it is a waste of memory. On embedded system (especially those whithout a file system :-)) this might be not acceptable.

2010-02-02 Reply Admin

The real wtf here is 'WTF do those magic numbers mean???' =D

2010-02-02 Reply Admin

you could eliminate the unconditional break with a tail-controlled loop as in

do { ... ... ... } while (false)

;-)

2010-02-02 Reply Admin

I think this is a useful pattern that might come in handy. Unusual, but not wtf.

frits · 2010-02-02 Reply Admin

patterns!!:
frits:
You write a base class that implements most of the logic. Then you create simple classes that customize or extend the base class. It's really simple.

No no no. Base class? Inheritance? No. You need interfaces! That way you can dependency-inject the strategy class from your XML file.
public interface MainTypeStrategy {
    public int getSubtype(int mainType);
}

public class MainTypeSevenStrategy implements MainTypeStrategy {
    public int getSubtype(int mainType) {
        return 4;
    }
}

public class MainTypeNineStrategy implements MainTypeStrategy {
    public int getSubtype(int mainType) {
        return 6;
    }
}

public class MainTypeElevenStrategy implements MainTypeStrategy {
    public int getSubtype(int mainType) {
        return 9;
    }
}


public class SomeClass {
    private MainTypeStrategy mainTypeStrategy;

    public int convertMainTypeToSubType(int mainType) {
        return mainTypeStrategy.getSubtype(mainType);
    }

    public setMainTypeStrategy(MainTypeStrategy mainTypeStrategy) {
        this.mainTypeStrategy = mainTypeStrategy;
    }
}
Wow! That was so easy. And maintainable. No one will ever be confused as to why I wrote so much code instead of an if-else block! Adding a new class in the future will be sooo much easier than adding an extra else-if line.

But seriously, patterns are good.... just not here.

That's not what I meant at all. Nor am I talking about any particular pattern per se. But I suspect you know that.

2010-02-02 Reply Admin

stuff:

Alan:

types = {
    7: 4,
    9: 6,
    11: 9,
}
if baseType not in types:
return None
return types[baseType]

Python FTW!

return {7: 4,
        9: 6,
        11: 9
       }.get(baseType, None)

Getting rid of the unnecessary conditional... FTW

STILL not semantically equivalent to the original code!

return {7: 4,
        9: 6,
        11: 9
       }.get(baseType, baseType)

2010-02-02 Reply Admin

FriedDan:
As for the WTF in the WTF, I'm not even sure why the else-if statement is relevant without a switch.
It could be written:
if(mainType == 7)
    subType = 4;
if(mainType == 9)
    subType = 6;
if(mainType == 11)
    subType = 9;
There's no potential overlap so no NEED for an else (even if it might make it more readable).

Except that we have no idea whether subType's type's operator=() has the side effect of altering the value of mainType. So you'd HAVE to break after each assignment...!

frits · 2010-02-02 Reply Admin

Maybe the "mainType" class has an overloaded "==" operator. If that is the case, then using a switch on "mainType" wouldn't be straighfoward. However, you could come up with something muy elegante like this .

2010-02-02 Reply Admin

As for the WTF in the WTF, I'm not even sure why the else-if >statement is relevant without a switch.

It could be written:

if(mainType == 7) subType = 4; if(mainType == 9) subType = 6; if(mainType == 11) subType = 9;

In this case, each if is tested. With an else-if or switch structure, it only continues to test up to the first true if statement.

2010-02-02 Reply Admin

Oh noes, but what will we do when we need to call MainTypeEightStrategy!?, obviously, you need to have a dynamic code generation system that parses XML thats retrieved via SOAP tunnelled over an encrypted SMTP relay, which generates new methods dynamically as they are called/needed, causing failproof code!

CAPTCHA: causa , like cause, but the dude to made it happen, "I'm the causa u playa' "

2010-02-02 Reply Admin

I just hate long blocks of flow control.

sub upgrade_subtype { 
  my ( $maintype, $old_subtype ) = @_; 
  return 4 if  $maintype == 7;
  return 6 if  $maintype == 9;
  return 9 if  $maintype == 11;
  return $old_subtype;
}

...

$subtype = upgrade_subtype( $maintype, $subtype );

I can't tell if thats winning the fail contest or failing the win contest.

CAPTCHA: Saepius, I guess like chimp royalty?

2010-02-02 Reply Admin

Quirkafleeg:
r3jjs:
If this code:
do { /*code here*/ } while (0);
was used in a #define, than considering the while(0) to be boilerplate doesn't bother me.
But the trailing ‘;’ in the #define is just plain broken: you'd have to remember to omit the semi-colon wherever you use the macro;; else you'll get errors.

An extra ; in C/C++ code is just an empty statement. You might want to try it out some day. A coworker of mine recently had some great fun in C# with if(...);{...}, wondering why the {...} block always executed.

Captcha: quis. Quissiness? Queasiness?

frits · 2010-02-02 Reply Admin

frits:
blah blah blah GIGANTIC!!!

Laughing Jack:
blah blah blah ENORMOUS!!!

quisling:
blah blah blah GARGANTUAN!!!

2010-02-02 Reply Admin

Wimps, all of you

subType = (int)Math.Round(1.25 * mainType - 4.9167);

BlueKitties · 2010-02-02 Reply Admin

I don't understand how this is C++ code at all. By all rights, this should have something similar to:

SubType subTypeObj = MainType::toSubType(mainTypeObj);

And again, if this is embedded or something, then they are mostly just writing C code with a C++ compiler. C++ is, afterall, a lot like several languages mushed into one. But that code snipped isn't "C++ish" at all, it looks like pain-jane C.

2010-02-02 Reply Admin

Ahruman:
Consultuning:
if( mainType > 6 && mainType < 12) {
    subType = mainType - 3 ;
    if( subType == 8 )
        ++subType;
}
This is in my mind way more legible and does exactly the same. Or I'm completely confused?
Two ifs? How abhorrently sloppy. Won’t someone _please_ think of the branch predictors!

	; v1 = mainType
	; v2 = subType
	SUB	a1,v1,#7	; -ve if <= 6
	CMP	a1,#4		; effective <= 11
	SUBLS	v2,v1,#3
	TEQLS	v2,#8
	ADDEQ	v2,v2,#1

Count the branch instructions :-)

2010-02-02 Reply Admin

Deadcode:
If you want to optimize for the fewest total number of operations (including conditional branches) without using a lookup table, and limiting division to that which can be optimized into bit shifts, how about this:
if ((unsigned)(mainType-7) < (mainType&1)*5)
    subType = ((mainType+3)*mainType)/16;
I believe it beats RandomNameOfHiding's solution, which takes a similar approach...

Which is optimal depends on the target CPU…

2010-02-02 Reply Admin

Deadcode:
Deadcode:
If you want to optimize for the fewest total number of operations (including conditional branches) without using a lookup table, and limiting division to that which can be optimized into bit shifts, how about this:
if ((unsigned)(mainType-7) < (mainType&1)*5)
    subType = ((mainType+3)*mainType)/16;
And here's one with even fewer operations and no multiplication:
if (mainType & (mainType-7 < 5u))
    subType = mainType + ((mainType-18)>>2);

Let's see… we can ignore the first condition, since (unsigned)(0-7) > 5u…

	SUB	a1,v1,#7
	CMP	a1,#5
	SUBLO	v2,v1,#18
	ADDLO	v2,v1,v2,ASR #2

TEH WINNAR! (Unless subType is unsigned…)

2010-02-02 Reply Admin

But where is multiple inheritance?

You just can't get true C++ elegance without it!

2010-02-02 Reply Admin

Annon:
Quirkafleeg:
r3jjs:
If this code:
do { /*code here*/ } while (0);
was used in a #define, than considering the while(0) to be boilerplate doesn't bother me.
But the trailing ‘;’ in the #define is just plain broken: you'd have to remember to omit the semi-colon wherever you use the macro;; else you'll get errors.
An extra ; in C/C++ code is just an empty statement.

I've highlighted the important bit for you. You might want to try it out some day.

A coworker of mine recently had some great fun in C# with if(...);{...}, wondering why the {...} block always executed.

Your cow-orker needs gcc's -Wempty-body (or -Wextra). <wanders off to use that>

DWalker59 · 2010-02-02 Reply Admin

JimTheJam:
For everyone who thinks switch/case statement are extremely superior to if-else construct (in this situation):
In the olden days (and maybe still) the case/switch statement checks the controlling variable against the lowest value, then checks against the highest value, then does an indexed GOTO using a table <<minor and major implimentation variations, depending upon your particular compiler and hardware>>

The resulting code is longer, slower, and more buggy (yes, I'm an old timer), than the nested if-then-else-if.

That is not possible unless the values are regularly spaced. If the values are 1, 2, 3, 4, 100, 59087, then an indexed Goto would not work.

2010-02-02 Reply Admin

tertiary:
Short and beautiful.
s = ((m == 7) ? 4 : (m == 9) ? 6 : (m == 11) ? 9 : 0);

ITYM "ternary". http://en.wikipedia.org/wiki/Ternary_operation

In computer science a ternary operator (sometimes incorrectly called a tertiary operator) is an operator that takes three arguments.

2010-02-02 Reply Admin

frits:
frits:
blah blah blah GIGANTIC!!!
Laughing Jack:
blah blah blah ENORMOUS!!!

quisling:
blah blah blah BORED!!!

FTFY...

(i lol'ed :D)

2010-02-02 Reply Admin

Deadcode:
Deadcode:
If you want to optimize for the fewest total number of operations (including conditional branches) without using a lookup table, and limiting division to that which can be optimized into bit shifts, how about this:
if ((unsigned)(mainType-7) < (mainType&1)*5)
    subType = ((mainType+3)*mainType)/16;
And here's one with even fewer operations and no multiplication:
if (mainType & (mainType-7 < 5u))
    subType = mainType + ((mainType-18)>>2);

Ahem. My LUT-based solution has the advantage of actually maintaining the semantics of the original code for all values of mainType – I used a suite of ten unit test to test every possible value except the 4294967286 uninteresting cases – while also fulfilling my completely arbitrary goal of avoiding control statements.

luis.espinal · 2010-02-02 Reply Admin

frits:
Any procedural solution for what this code is attempting would be smelly. However, I must say this is probably one of the ugliest ways to do it.

Why just procedural? I've seen ugly crap like that in OO code. Knowledge of fundamental control structures (or lack thereof) are independent of the programming paradigm.... just saying.

Other than, that is one ugly code snippet.

2010-02-02 Reply Admin

quisling:
Laughing Jack:
Indeed. This is why I find your so amusing.
I think you accidentally your smug there.

Yes, yes I understand. I'll be in the study.

[BANG! Thump.]

frits · 2010-02-02 Reply Admin

Just procedural?

Else... where?

Leave a comment on “Else... where?”