| « Prev | Page 1 | Page 2 | Next » |
|
Hi Alex.
|
|
Wow... I... uh... wow....
So he's basically looking for a 10-character ISBN, the first 9 of which are guaranteed to be numbers, the last can be either a number or X. This could have been done so much simpler it's shocking. It reminds me of that code a little while back where the guy made a <select> tag in HTML by hand and manually entered all the possibilities for hours and minutes. I can honestly say I've never used "GoTo", I avoid it like the plague or a hooker with herpes. I love how he has a GoTo that takes the code to the next line anyway, and at the very end he has "Exit Function". Good thing he put that in there, because the ambiguous "End Function" right afterward is unpredictable. And correct me if I'm wrong, but if you pass the parameter as a string, won't IsEmpty() return True if the Len() = 0? That situation would already be caught with the line "If Len() <= 9" (which could have all been simplified to "If Len() <> 10") Maybe he just didn't know VB supported loops? Maybe he had a head injury? |
|
Actually in defense of the original author of the code .....
nah, can't do it. What was I thinking?!? |
|
it... runs... fast ?
then again, I should use the IF statement instead of CASE. |
re: To ISBN or not ISBN, that is the question
2004-09-21 05:02
•
by
Martin DeMello
|
|
<i>
GoTo TheEnd TheEnd: # ........... Exit Function End Function </i> Looks like someone doesn't trust the program counter :) |
|
Uh, this must be a fabrication. I simply refuse to believe that there are people as dumb as this one. The intelligence displayed in this piece of code isn't enough to stimulate automatic breathing. I've heard of nobody programming while in a respirator.
|
|
Damn - I really was stunned for a moment... When I saw the first Select Case appearing on my screen, I was like "oh no - he didn't"... and stayed like that for 5 minutes...
What I really do like about this code is his errorhandling... What if the CPU just jumped from it's own to one of the labels? Well, hell must have broken loose then, and he raises the error 666... Good thinking, can't be safe enought now can we? Just missing the "PrayForMerci" label... Also, his errormessages are so clear we should really make a difference between the 2 error cases. Also when something went wrong, you really couldn't return "no", now could you? Also - booleans were invented only for educational purposes... |
|
It's obvious this code is the root of all evil ;)
* Error Code 666 ... * 9 Identical select blocks * ... Select Blocks for finding digits?! * GoTo is considered "unclean" * String return value when boolean could suffice. (...) |
|
I would pay to see what the first version of this code looked like !!!!
Kind of like when you drive the down the road and see a car accident, you can't help but look... the first version was probably: If ISBN ="000000000" or ISBN ="000000001" or ISBN ="000000002" or ...etc... over a billion lines of code !!! |
|
I have to join the (surprisingly small) group of readers who cannot believe that someone could actually be dumb enough to write code like this.
Actually, I "coded" ASP/VBScript for a full week before discovering loops (quite a while back ;-)... Makes you wonder just how long this guy has been in the game! |
|
'Init Variables
programmerSucks="yep" isGoodForTheIndustry="no" shouldbeFlippingBurgers="yep" |
|
I am not going to blame the programmer for the GOTO stuff. It is how try-catch-finally works in VB.
But after a year and five months and four versions, urrr... |
|
Richard Tallent: "*This* is why I've petitioned Microsoft to rename VB.NET to B#"
I think Kernighan and Ritchie would have cause to sue. B Sharp is C. |
re: To ISBN or not ISBN, that is the question
2004-09-21 14:24
•
by
Christian Romney
|
|
I have a really hard time believing someone wrote this in any system. Maybe they submitter just wanted to get a rise out of us. The give away is as variant. They know about selects and strings and never heard of booleans? Still, even if it is a hoax, it has entertainment quality.
|
|
Too stupid!? Puleezzee ...
I hate to bring politics ... but only 41% of people in recent Newsweek poll said that Saddam was not "DIRECTLY involved in planning the attacks on 9/11" (exact wording of question, including empahsis). That's nearly 60% of Americans who are complete morons! Surely one or two of them are coders? |
|
Given that he's using goto (which I'm not defending) the "goto TheEnd" just before the label is actually good practice, because otherwise adding another block and forgetting to add the now-necessary "goto" would introduce bugs.
My major whap would be the function name. OrNot? |
|
Where's the line numbers? He's obviously quite comfortable with BASICA style coding.
"+1 Internationalization for supporting encodings where [0-9] are not contiguous (do these exist?)" I doubt that's an issue. An implementation not smart enough to expand [0-9] into the set of roman numerals independantly of charset would probably just use latin-1 anyway. I bet the checksum function would be called CheckIfPossibleISBNCheckdigitIsCorrectISBNCheckdigitForISBN(PossiblyAnISBN) Hey, at least he doesn't have: Function CheckIfPossibleISBNCheckdigitIsCorrectISBNCheckdigitForISBN(PossiblyAnISBN as String) as variant CheckIfPossibleISBNCheckdigitIsCorrectISBNCheckdigitForISBN = "yup" Exit Function End Function |
|
<i>
So please, send in your non-BASIC code snippets and the like -- if for no other reason than to make me feel less bad about my language of choice ;-). </i> <pre> # Perl solution sub CheckIfISBNIsGoodISBNOrNot { my ($possibleISBN) = @_; my ($isbn) = ( $possibleISBN =~ /^\d{9}(\d|X|x)$/ ); unless( $isbn ) { return 'no'; } return 'no'; } </pre> |
|
@perlist: Uhm, shouldn't that last "return 'no'" be "return 'yes'"?
Or are you going for a WTF yourself? |
|
We obviously need to take away this guy's Ctrl, C and V keys and lock them in a box and then weld it shut and then cast a sealing spell on it and then place it on a rocket and then fire it into the sun and then blow up the sun and the universe...
|
|
Maybe there's a clue in the error number: 666
diffult to think about good code while you're chasing all those souls thru hell |
|
Hey the code was done in the year 2000.. so worst comes to worse.. it is at least Y2K compliant!!
|
|
I've only just discovered this site, but has no-one else noticed that it will accept any string of 10 chatacters that ends in [0-9X]
stupidtitX is a valid string for this routine as it overwrites its detected errors with the next character This is really really bad |
|
@Peter
Where does it overwrite its detected errors? 'yup looks like it's just a comment. (I don't know VB.) The code does continue to check the following values, but I don't see where it would overwrite the errors. |
|
I don't know about current versions of VB, but what I remember of VB3 (before abandoning it), is that On Error handlers were required to explicitly exit the error block using either a Goto an Exit Function, and possibly by something else. In any event, they weren't allowed to just drop out the bottom. Either later versions of VB relaxed that, or this might be old code from VB3 days.
|
|
"<i>
So please, send in your non-BASIC code snippets and the like -- if for no other reason than to make me feel less bad about my language of choice ;-). </i> <pre> # Perl solution sub CheckIfISBNIsGoodISBNOrNot { my ($possibleISBN) = @_; my ($isbn) = ( $possibleISBN =~ /^\d{9}(\d|X|x)$/ ); unless( $isbn ) { return 'no'; } return 'no'; } </pre> " Shouldn't that first line be: my ($possibleISBN) = shift; as you end up with $possibleISBN being equal to the number of elements in @_... |
|
One line of code, some explanation because it's not readable for those who don't know regexps. I left the method name alone, though it hurt, but had to change the return value to boolean to avoid throwing up.
Did I miss any test cases? def CheckIfISBNIsGoodISBNOrNot(possibleISBN) # An ISBN is apparently made up of nine digits followed by either a tenth, # or the letter 'X'. !/^\d{9}[\dX]$/.match(possibleISBN).nil? end if __FILE__ == $0 require 'test/unit' include Test::Unit class ISBNTest < TestCase def test_empty assert(!CheckIfISBNIsGoodISBNOrNot("")) end def test_too_short assert(!CheckIfISBNIsGoodISBNOrNot("000")) end def test_kinda_random assert(!CheckIfISBNIsGoodISBNOrNot("asdsaoidaiodjasjdasd")) end def test_too_many_chars assert(!CheckIfISBNIsGoodISBNOrNot("00000000000X")) end def test_dash assert(!CheckIfISBNIsGoodISBNOrNot("0000-0000X")) end def test_ok_with_x assert(CheckIfISBNIsGoodISBNOrNot("000000000X")) end def test_ok_no_x assert(CheckIfISBNIsGoodISBNOrNot("0000000000")) end end end |
|
'yup
Um... OK. Way to go, clock-milking retard! |
|
# Yet another Perl solution
sub CheckIfISBNIsGoodISBNOrNot { (($arg = shift) =~ /^\d{9}(\d|x)$/i) ? 1 : 0; } But really ... |
|
# Perl solution #3 [even simpler version of the previous one]
sub CheckIfISBNIsGoodISBNOrNot { ((shift) =~ /^\d{9}(\d|x)$/i) ? 1 : 0; } |
|
patrick: no, ($x)=@_; is a list assignment.
Folks with the regexes, whats with the alternations? char classes are more efficient. sub CheckIfISBNIsGoodISBNOrNot { $_[0] =~/^\d{9}[\dxX]$/ } |
|
It was on PHP (rollseyes). Don't get me wrong, PHp is a nifty (or actually powerful = dangerous?) language.
Anyway. I think i can solve the mystery behind this: COPY/PASTE! Some programmers are so lazy that they rather copy/paste hundreds of lines of code instead of just adding some if's. So this guy probably did one line of code, copied it 10 times, changed the numbers, copied it again 5 times... etc. This is the result. Well, the programmer got praised for doing a heavy database management system in 5 minutes. But boy, oh boy the horrors I felt when I had to MAINTAIN IT!!!! |
|
I particularly like the comment masterpieces. What literature!
ErrorChecking: 'Check errors :-D |
|
+1 Point for readability! -4000 points for stupidity. I sincerely hope no one defends this code.
|
|
My mistake, IsEmpty will always return False on a string, as long as it's Dim'ed or passed as a parameter. Either way, IsEmpty is useless in this context.
|
|
Scary that this went through 5 versions and is still like this.
There are just hundreds of ways this could be done better. I love the error messages. Very descriptive to the person calling this "beast" of a function ;) |
|
I love the end of the function:
Exit Function End Function I guess sometimes you REALLY have to be sure you don't walk past the end of the function... :) |
|
Does VB support regular expressions? If so, this is yet another one-liner that didn't happen because the instruction manual is still in the shrinkwrapped box. :)
|
|
I especially like if <= 11 and >= 9 when you want something that is a length of 10... Lets you know right off the bat that you are in for a treat.
|
|
Guess he never heard of Regular Expressions
lol @ "it burns", that made me laugh for a few minutes |
|
How come the error number is '666' ? Is this the work of the devil? ;-)
|
|
Bu and Steve: VB6 doesn't support RegEx out of the box... you actually have to use the VBScript runtime for that.
One time on a vb6 project, i needed to do a regular expression to simplify 30 or 40 lines of code down into like 3 lines of regular expressions... and my manager forbid me from doing it because he didn't want to include the VBScript runtime in the setup package. |
|
Must be another example of <A HREF="http://thedailywtf.com/archive/2004/09/17/1844.aspx">consultant code</A> where every line of code equals more money!
|
|
And after all that, it doesn't even check the checksum! Perhaps the programmer tried to do it with Select and gave up when the code grew too long for the VB editor or compiler to handle.
|
|
Most VBers don't know regular expressions....
|
|
If (aValue <> "no") And (aValue <> "yes") Then Err.Raise 666, ,"SCREW YOU! I DON'T KNOW WHAT THE F**K HAPPENED!"
|
|
Alex, as for non-VB code, check your inbox...
|
|
This is how I would have done it...
Private Function isValidISBN(strISBN As String) As Boolean Dim regex As New RegExp regex.Pattern = "^\d{9}(\d|X)$" isValidISBN = regex.Test(strISBN) 'And isValidCheckSum(strISBN) End Function 'download MS windows script hear : http://msdn.microsoft.com/library/default.asp?url=/downloads/list/webdev.asp |
|
Actually isValidCheckSum(strISBN) in above code should be hasValidISBNCheckSum(strISBN)
|
| « Prev | Page 1 | Page 2 | Next » |
