- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
So if selected > 3 you'll set index to a value greater than 3? As opposed to the original code which set index to 0.
Admin
Good, because it's wrong.
Edit: Bah, Ben beat me by a few seconds...
Admin
[code]private int GetSelected( int selected ) { int constant; constant = -6;
int firstPower; firstPower = 11; firstPower *= selected;
int secondPower; secondPower = -6; secondPower *= selected; secondPower *= selected;
int thirdPower; thirdPower = 1; thirdPower *= selected; thirdPower *= selected; thirdPower *= selected;
int sum = 0; sum += thirdPower; sum += secondPower; sum += firstPower; sum += constant;
if (sum == 0) { return selected; }
sum -= sum;
return sum; }
Admin
Wow, very nice... I like it. Clever.
Admin
Use Ruby.
def get_message(selected) (1..3).member? selected ? selected : 0 end
Admin
Another Ruby version.
def GetMessage(selected) classDef=<<-END_OF_CLASS_DEF class GetMessageClass def GetMessageMethod(index) if(index<0 or index>3) return 0 else return index end end end END_OF_CLASS_DEF eval(classDef) gmc=GetMessageClass.new mthd=gmc.method("GetMessageMethod") return mthd.call(selected) endAdmin
Use Ruby.
def get_message(selected) (1..3).member? selected ? selected : 0 end
Admin
You can write that in just one line:
private int GetMessage( int selected ) { return selected; }
Admin
The Incompetant, of cause (Refer to the Contractors Note)
Admin
... I meant to quote this with my previous post.
Admin
I'm going to port this over to PHP. Every good code monkey knows that this function would be easier to read if it was OOP...
<?php require_once('class.message_handler.php'); class Message extends Message_Handler { private $_xLoopStartPointVarInt; private $_errFlagOnIntInitFromFunction; private $_loopValidCounterNoErrReturn; private $_tempValidTestVarInt; private $_tempVarCheckValidNegativeInt private $_tempValidTestVarInt; private $_finalCheckVarValInvalid; public function GetMessage( switchWhichNumToBeReturnedFromThisFunction ) { if ((switchWhichNumToBeReturnedFromThisFunction >=5) || (switchWhichNumToBeReturnedFromThisFunction < 0) || (!is_numeric(switchWhichNumToBeReturnedFromThisFunction)) { $this -> _errFlagOnIntInitFromFunction = true; } else { // verify that the number is valid $this -> _xLoopStartPointVarInt = 0; $this -> _loopValidCounterNoErrReturn = 0; for ($x = this -> _xLoopStartPointVarInt; x<=5; x++) { $this -> _loopValidCounterNoErrReturn = $this -> _loopValidCounterNoErrReturn +1; } if ($this -> _loopValidCounterNoErrReturn >=5) { $this -> _tempValidTestVarInt = false; } $this -> _xLoopStartPointVarInt = 0; for ($x = this -> _xLoopStartPointVarInt; x<=0; x--) { $this -> _tempVarCheckValidNegativeInt++; } if ($this -> _tempVarCheckValidNegativeInt >0) { $this -> _tempVarCheckValidNegativeInt = false; } if (((((!$this -> _tempValidTestVarInt)) || ((!$this -> _tempVarCheckValidNegativeInt))))) { // flag as invald for debugging $this -> _finalCheckVarValInvalid = true; } else { return (((switchWhichNumToBeReturnedFromThisFunction)*2)-1); } } }Admin
Okay, I decided to do a count of how many people wrote a buggy version of this function. Here are my counts:
50 -- The number of correct solutions. I didn't count unique solutions or even unique submitters; I just went through and looked at all the posts that had solutions in them that weren't talking about other people's.
35 -- The number of solutions that I don't know if they work. These usually involved either more thinking than I wanted to put in at midnight (often a lot of bit twiddling) or were in languages that I didn't know and didn't feel like trying to figure out. (Some of the 50 above were also unknown, but I could get a good enough sense.)
4 -- The number of solutions that are correct except for some trivial mistake. For instance, one post uses = instead of ==. (Or otherwise screws up to make getMessage(0) undefined.)
7 -- The approximate number that invoke technically undefined behavior in C, even if it's pretty well known what it'll do. For instance, underflowing an integer or casting signed to unsigned. (A couple of the correct solutions should probably be in here.)
12 -- The number of solutions that are correct only for the numbers 0-3, or some other set. (For instance, there was one correct for all numbers except -3 to -1, which were mapped to their absolute values.)
2 -- The number of solutions that are correct for positive numbers, but not for negative numbers. (There might be an undefined one here too.)
8 -- The number of solutions that are otherwise wrong. I think this means that it maps 0-3 to the wrong values, but I don't remember all the instances and can't guarantee I was consistent. (For instance, one maps 0->4.)
In other words, if you count the undefined behavior ones as wrong and the almost correct ones as correct, 29 out of 83 people -- 35% of answers that I graded implement the function wrong. Even if you assume that all the ungraded answers are correct, that's still 29 of 118 or 25% of answers are wrong.
There's your WTF.
Admin
Bollocks. That's what proper specs, unit tests, and software testers are for.
private int getMessage(int selected) { return Math.random()%4; }Admin
# cashmachine.py output = 'private int GetMessage( int selected )\n{\n\ \tint result = 0;\n\ \tswitch(selected)\n\t{\n' case_head = '\t\tcase ' case_body = ':\n\t\t\tresult = ' case_tail = ';\n\t\t\tbreak;\n' i = int_min while (i <= int_max): if i < 0 or i > 3: output += case_head + str(i) + case_body + '0' + case_tail else: output += case_head + str(i) + case_body + str(i) + case_tail i += 1 output += '\t}\n\treturn result;\n}\n' ofile = file( 'GetMessage.c', 'w+' ) ofile.write( output ) ofile.close()Admin
I don't know how easy it is to translate to x86. It works because Shift Right Arithmetic Word Immediate copies the sign bit of r3 into every bit of r4, then the other two instructions calculate the two's complement if r3 is negative or do nothing otherwise.
Admin
Very simple ..
private int GetMessage( int selected ) { return selected; }
[email protected]
Admin
I do not why you make this function as Case and Index is same case 0: index = 0; break;
So we you change this function to following code, it will return same result.
private int GetMessage( int selected ) { return index; }
Harry [email protected]
Admin
This really calls for a Duff's Device:
private int GetMessage( int selected ) { int index=0; int n=(selected+2)/3; if (n<2) ....{ ....index-=3; ....switch (selected%3) ........{ ........case 0: do { index++; ........case 2: ...... index++; ........case 1: ...... index++; ........................} while (--n>0); ........} ....} return index; }
(please ignore those dots, they are just for indentation)
Admin
Sweet. How elegant. Any more of this ?
Admin
Why did you quit leaving the job refactoring half done ? IMHO, evil like this needs eradicated completely.
Admin
As far as refactoring is concerned, there is not need to write this function. One can use int Selected as it is where he/she wants int index to be.
Anyhow
private int GetMessage(int selected) { return (selected>=0 || selected<=3?selected:0)
}
Admin
I think we're missing a few languages here; I'll add two of them.
PostScript:
F77:
Admin
How you might do it with switch:
private int GetMessage( int selected ) { switch( selected ) { case 0: case 1: case 2: case 3: return selected; default: return 0; } }For those who don't like returning from the middle of a function you could start with index=0 and skip the default altogether with "index=selected" for the first 3 cases.
Admin
There is an interresting sqrt in the Quake engine. With an even more interresting comment on it (Dunno if it was cleaned, but it was in the leaked version). Cant remember it now, but I seem to remember some fancy bitshifting and stuff. And the comment were: //wtf?!
Admin
the real wtf is amount of noobs in here, who can't write it right. no wonder world is full of wtfs when half of readers of TDWTF.com are wtfers
Admin
What about an OO pattern-wise implementation?
That's it...
public class GetMessageImpl { public int getMessage(int selected) { CommandInterface command=CmdFactory.getCommand(selected); return command.getMessage(); } }
public class CmdFactory { public static CommandInterface getCommand(int i) { try { return Class.forName("Command"+i).newInstance(); } catch (Exception e) { return new CommandDefault(); } } }
public interface CommandInterface { public int getMessage(); }
public class CommandDefault implements CommandInterface { public int getMessage() { return 0; } }
public class Command0 implements CommandInterface { public int getMessage() { return 0; } }
public class Command1 implements CommandInterface { public int getMessage() { return 1; } }
public class Command2 implements CommandInterface { public int getMessage() { return 2; } }
public class Command3 implements CommandInterface { public int getMessage() { return 3; } }
I'm thinking about an EJB solution.....
Admin
Hurrah
int getMessage(int c){ return c*!(1-!(c>>(!!('<'-'>')["<<"]<<!!('>'>"<]"[!'>']))))&3; }
Admin
is Omg Object Management Group (CORBA etc) or Oh My Gosh!
We don't know the context here, however I would say that a function called GetMessage should get a message, not a number between 0 and 3.
Admin
dammit, i can make it more efficient:
int getMessage(int c){ return c*!(1-!(c>>(!!('<'-'>')["<<"]<<!!('>'>"<]"[!'>'])))); }I have now saved that nasty & :D
Admin
What's deterministic about abs? It has to check the sign. You need to use sqrt((a-b)(a-b)) instead.
Admin
The way Im reading it selected is a string, seeing as the function returns an int I can assume that the user simply didnt want to/know how to cast selected to an int.
could probably be summed up as:
int index = int.parse(selected); if ((int < 1) || (int > 3)) { index = 0; } return index;
I know that the if could be shortened, but this is "good enough" (TM)
Admin
int GetMessage(int value) { int index = -(valuevaluevalue - 6valuevalue + 5*value - 6)/6;
return (index == value) ? index : 0; }
Admin
Admin
(selected &= 3);
// Any better ?
Admin
Yes, but look how easy to read it is, and how clearly you can see that it is a useless function that just assigns the value of the "selected" int indicating the message to the variable "index". The whole function can be replaced by one line wherever the function is called:
index = selected;
Where the contractor can really make the big bucks, and be applauded, is in in the accompanying 2 pages of documentation that describe the design decision to use the standard "index" variable across all programs. For every line of code there should be at least 5 lines of documentation, right? :-)
Admin
It's nice. Until you start handling big numbers...
Admin
//the program
main { } //I'm great!
Admin
yeah, what about something not buggy noob?
captcha:muhahaha, how fitting
Admin
It also won't compile. ;)
Admin
return ((selected & 0xFFFFFFFC)? 0 : selected);
or, without the branch:
return selected * (((selected & 0xFFFFFFFC) / (selected & 0xFFFFFFFC)) ^ 0x1);
I have no doubt it can be done faster :)
Admin
From Wikipedia: "In computer science, a deterministic algorithm is an algorithm which, in informal terms, behaves predictably. Given a particular input, it will always produce the same correct output, and the underlying machine will always pass through the same sequence of states"
abs(4) always equals 4. abs(-3) always equals 3. Having branches DOES NOT EQUAL nondeterministic. The following is deterministic:
int abs(int x) { if(x<0) return -x; return x; }Then you can switch to the xor version, also posted above: a = a^b; b = a^b; a = a^b;
Also, I never worked this problem through to completion, but I think on 2's complement architectures at least the over and underflow works such that the arithmetic version will still work. Could be wrong about that though.
Admin
Very well put. I've only been reading this site for a week or two, but I'm amazed at the poor S/N ratio on here.
I've been inheriting other peoples' code off and on for two decades now, and the original code smells like either: (a) a stub itended to get other processing code in the 1-3 branches, or (b) a routine that got reworked over time to no longer have meaning, yet has remained for purely org reasons like fear of unintended side-effects from cleaning it up.
My money is on (a). The rest of this thread is an exercise in hand warmers.
Admin
asm("xchgl %%eax,%%ebx" : "=a"(a), "=b"(b) : "a"(a), "b"(b) :);The advantage over swap(a, b); is that a and b don't have to be of the same type. Although, in the wrong hands that would probably be a disadvantage.
Admin
You're all missing the real purpose of the code.
Sure, for now it looks silly, but what you've missed is that it provides an excellent customization opportunity.
The code that says:
case 1: index = 1; break;
will at some point be replaced with:
case 1: index = 1; if ((lastIndex == 12) || (employeeCount == 3)) index = 2; // code was wrong in these cases
Admin
Because I was being paid half-a-pittance, and after refactoring, I still would have been left with refactored crap.
Admin
private int GetMessage( int selected ) { return (selected & 3) == selected ? selected : 0; }
Admin
Amazing that after 5 (FIVE) pages of commentary, people are still missing the basic logic of the initial post and posting flawed code. I have to agree with others here: the real WTF is all the folks posting on here that look down their noses at some poor, out-of-context example and then return to their jobs where they most likely write code just as flawed as what they post here.
Admin
Pascal:
Hope I still remember it correctly.
Admin
@Maks Verver
might be faster (in some cases, but i'm not sure):
int GetMessage(int s){ return ( ((unsigned int)s < 4) ? s : 0); }
Admin
Another Pascal solution, and rather close to the original code:
function getMessage(Selected: integer): integer; begin case Selected of 0-4: Result:=Selected; else: Result:=0; end; end;