- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- 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
if (state='WTF') return FRIST
Admin
I'm always glad that here in Holland (The Netherlands for you, geeks...) we only have twelve provinces to code for.
Admin
Oh man...the state of that code...
Admin
Never mind Guam, how about DC?
Admin
Ok, it's a long list of ifs and you could refactor it to a couple of lines of code but other than that, what's so wtf-worthy about it?
Admin
REAL programmers don't use lists and indexOf() (or hashmaps)
Admin
It's a long list of ifs and you could refactor it to a couple of lines of code - and it's just so wrong.
(CAPTCHA: capio - a device used on a guitar to change the key of a song without actually learning how to play the chords. "Those chords are too hard - I'll just capio up 2 frets")
Admin
Admin
Well, apart from the obvious:
Admin
That makes sense.
Admin
You'd still need to build the dictionary/map, so the code wouldn't be that shorter.
I think TRWTF is returning 51 when the value isn't found.
Admin
And the even more obvious: it's private function. So you can create objects all you want, but you still cannot access the function.
Admin
District of Columbia is also not represented, of course, but the WTF is using linear search and not some kind of map or other efficient lookup.
(Or constant time in table of just 676 entries of course, converting to upper-case if you desire and if it isn't a letter it isn't found).
I would use 0 for not-found and 1-N for the found ones, or -1 for not found possibly.
Either way, you do still have to build the table, albeit you might decide not to "hard code" it and read it from config, just in case the USA declares a new state at some point before your next release.
Admin
Admin
See the obvious: This is a private function of this class, which means it serves for some well defined purpose. This class knows that 51 is an error indicator. And the numbers do not have any meaning outside the class or even for inheriting classes.
Not so obvious: Is this really intended or just done mistakenly?
Admin
[quote user="Muzer Well, apart from the obvious:
And it shouldn't be static. Because there is a "this" - the class that stores your lookup table.
Admin
I presume the "WTF" grade here represents "Worse Than an F"?
Admin
Admin
Smaller WTF: Not using the the FIPS state codes. That list already has reserved numbers for all of the territories that could someday become states.
Admin
This is still ugly but better. Not sure if the syntax is correct for this language but you get the point
Ugly but constant time and uses an enum rather than magic numbers.
I'd rather something like this in C++ but not sure how feasible it would be in other languages
Admin
Surely 51 represents the UK, the 51st State of America.
plaga
Admin
52nd. Canada is the 51st
Admin
In the old days, this would be inefficient and banned for performance reasons. And it duplicates all those "return"s when it could just "go to" a single return at the end.
But with prices falling on modern hardware, and the astronomical programmer salaries, readability is more important than performance. You should be able to see at a glance what it says. And this says you need a new programmer.
Admin
My first thought, the real WTF is that the default value is for Great Britain
Admin
The US, on the other hand...
Never mind.
Admin
The west of the Netherlands is the part where 'hospitality' means that you get one (1) dry biscuit (not a cookie) with your cup of coffee.
Admin
[quote user="Cbuttius"][quote user="Muzer] Well, apart from the obvious:
And it shouldn't be static. Because there is a "this" - the class that stores your lookup table. [/quote] There isn't, though, not in THAT code. I don't really think it should be static, just that the code should save the result into some var (though I don't know what the rest of the state class is like). [quote] [quote]
Not found isn't necessarily an error, and they are enumerations. Using an enum would probably be tidier, and a more efficient algorithm for string text to enum. 51 is a perfectly good number of "not found" as an enumeration albeit I would possibly use 0, however it is highly unlikely this list is ever going to change.
The software is presumably written for internal use and not for international usage so countries that don't have 2 letter states or have a different number of them are irrelevant. Coding for this scenario would be a waste of developer time unless you intend to sell the software.
[/quote] Yes, of course it would be perfectly fine - if it actually used an enum. Currently, you just have to remember when writing any function that uses it that 51 = not found. Not good.
I never mentioned anything about international use.
Admin
Won't the end result be exactly the same (unless the compiler is very dumb)?
Admin
I've seen some kludge like this where the "developer" used a property getter to return the value based upon another property (e.g. get {return get_state_index(this.StateAbbreviation)}) - you just hope that they set the referenced field correctly.
Admin
This problem is a perfect match for the FOR_CASE paradigm.
Admin
This WTF isn't a real WTF. Not in the way it is presented anyway. The method is private, so either it is for some debugging purpose or it is called via a public wrapper, that could just happen to be static.
ie:
Admin
(Truth: Atari BASIC used two-byte branch instructions instead of three-byte return instructions, to branch to a return point that was shared by several subroutines. Now that's geeky! And even with that, they came within about seven bytes of running out of the 8K available.)
Admin
Admin
Surely instead of returning an error value that isn't much different from a regular value, it should have done
throw new NotAssimilatedYetException()
Admin
I don't like all those magic numbers.
Admin
When did it become fashionable to say "ho" instead of "ha"???
Admin
return index('ALAKAZARCA...', $state) / 2;
Admin
Admin
Including Puerto Rico isn't all that strange... in the banking industry it's been a common theme in most of my jobs. If you want strange consider my first finance company that had two divisions, domestic and international. Not all that unusual unless you consider that Hawaii was part of the international division yet Canada was part of the domestic division.
Admin
Admin
Everyone missed the obvious. MI (listed twice) is not the abbreviation for Mississippi. So, 51 isn't error, so - quickly - which state is ACTUALLY missing?
Admin
Admin
(Actually, nobody will admit to being from L.A. The closer you get to the center of town, the more obscure subdivisions there are so people can say "I'm from Flagpole Heights" or whatever instead of admitting to being from L.A.)
Admin
I'm not American but I think LA is Louisiana?
Admin
Admin
Admin
Admin
More over Why convert to an arbitrary index in the first place?
Admin
The second "MI" is actually "MT" (Montana). I had to bump up the text size to see the difference.
Admin
Must be The Weather Channel. During the Hurricane Isaac coverage, they wouldn't say "Mississippi," they'd call it "the land mass between New Orleans and Mobile"--at least until tens of thousands of Mississippians started making fun of them on Facebook and Twitter. Then they decided Miss. was a state after all.