- 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
Wow. Just... Wow
Admin
Speaking of WTFs, who's the idiot that designed the FindFirstFile/FindNextFile interface in the first place? Yeah, that's real convenient to use in a loop, you fuckhead.
As for whoever wrote today's snippet, I can only quote from Super Troopers: "We should've taken him out the back and shot him a long time ago."
Admin
Sounds like a bad case of shell programmer's disease.
Admin
My C++ is rusty, but I'll take a stab at it:
Admin
Also,
printf("Too Many Files to Process! Please select fewer files!\n");Admin
"Files[i][0]" ensures that the next file entry is an empty string. Ie, first character of the name is set to 0.
I assumed anything not stated was correct (buffers are large enough, argc was tested). Still lots of icky left.
It took me a while to figure out what it was parsing (rarely use DOS shells when perfectly good Cygwin is available). Then I was sure it was leaving a stray CR behind, until I remembered that text mode is the default for fopen(). I never use that sort of stuff on Windows.
The big gotcha is that directories and file names may have embedded spaces in them. Whoops! Other stuff is just your basic mediocre programming style (bugs with corner cases, etc.).
Too bad standard C didn't provide directory reading routines.Using FindFirstFile will work but is unportable.
"DIR /B" gives a much easier parsing format, but is still too much of a "I didn't bother checking if there were routines to do this for me" style.
Admin
I'm with hank on this one, sorting a date in YYYYMMDD format is easier that sorting a date in DDMMYYYY or MMDDYYYY. And also for the international community, it is easier to read.
as for everything else the problem is the guy who wrote this doesn't know the stdc libary that well.
he could of just used the dirent interface which is platform independant. (i think, wrote a simple program using the interface in Borland C++ and it worked) .
Admin
dirent is a Unixism. There is no standard way to do directory operations in C or C++.
Admin
This is true (for now), but there are heavily portable ways to do it.
Huh?
Admin
For someone who doesn't know about "do {} while (bool)", yes it would be a pain. But for those of us who do, it isn't a big deal.
Admin
LMAO!
The probable reason it didn't work is that since system("") issues OS command it doesn't wait until it finished but continues down in the code hoping that the file as already filled with all files. Yet another toys'r'us programmer.
Admin
Uh, system() is a blocking call. Get back to stacking those Elmo dolls.
Admin
The ISO standard is actually YYYY-MM-DD, which is both easy to read, maintain sortability and unambigouness. What <i>really</i> bothers me is how it is possible to come up with the MM/DD/YY date format idea in the first place.
- Jon
Admin
At least on my computer getting the path already fails:
Just to give you the idea:
> Dir
Datenträger in Laufwerk Z: ist DATA
Datenträgernummer: 14DB-C6D8
Verzeichnis von Z:\
It took me quite a while to figure out, what "tmpstr[1] == 'D'" was supposed to do... :-)
Admin
Are you actually being serious? If so, your programming knowledge let alone API knowledge is severely deficient!
Admin
Admin
In English the usual way to write today's date in full is "fourteenth of November 2006", or more often "14th November 2006". But the US dialect, at least when spoken, uses "November 14 2006" instead. No idea why though unless it is just another Brewsterism.
Admin
I haven't tried to analyze the code, but ofcourse the REAL WTF here is:
I wonder why the job they had to do was so special that it required a custom programming language.
Admin
What's wrong with that format? It allows easy sorting of files by name in a directory system for one...
CAPTCHA: giggity
Admin
std::system() is blocking, otherwise how would it return the executed command return status ?
Admin
Well enlighten me, because I'm still in the dark here... Is it to test for a directory?
Admin
OMG. My c days are long past, and (hopefully) not too far in the future, so it took a few minutes to figure out what he's doing.
Ahem. For an in-house, always my own, never to be seen by anyone else app, that might pass the "use-it" test. But when Some Ass assumes that it will always work, and if it doesn't we can fix it, my teeth ache.
On the other hand, what if that was never supposed to get out in the wild, but some manager decided that it was "good enough".
Never mind. WTF.
Admin
> If anything, it should read 'Choose a Directory with less files'. Don't think anyone's selecting any kind of file...
_Fewer_ files, since they're countable. "Less" files == smaller files :P
Admin
Oh yeah, I guess you are one of those guys who think time should be written on the format mm/ss/hh? :-)
Admin
Ok, I think the biggest WTF is to ask if you can see why this "code" didn't work. I mean, WTF, this actuall DID work ???
Admin
There are more poblems in that peice of code then in M$ windows(and we all know how many poblems there are in windows).
Admin
Admin
Admin
That shows the WTF, doesn't it? There are two places where you have to check if your handle is valid, if your file is of the right type, if there aren't any locks on it, etc.
Admin
Johnny Christmas, that's a WTF right there. Why the hell are you trying to do clean-up/termination in the loop increment expression? It's just confusing, prone to error, and ugly. Put the FindClose() after the loop, and it works and requires no thought to understand.
Use a do-while loop as mentioned above - it's what they're for, ffs.
I hate people trying to be tricksy with for loops. But then I've had people do this:
for (i = 0; i < 40; i++) if (i % 2)
{
// Do stuff
}
Only the for loop part was so long and complicated, that the 'if' conditional was off the right-hand side of my screen, so I may be biased.
Actually no, your code sucked. No other way to describe it.
Admin
I reckon that if the code was working at some point, then the problem could lie with something preventing creating the temporary file... Either disk space or permission... Or as this is dependent on the output of the dir command, if the layout changes, with a low key parser like that, you're immediately stuffed...
I'm not suggesting doing anything more complex, this is just the wrong way to do it...
Parsing the result of system command in temp file is just the most abject work around to not knowing (not bothering looking) for the corect api...
Admin
Think about what you're saying (i.e. write the full code in your head, not just the easy bit). With a do loop you have to check the return code for INVALID_HANDLE_VALUE in two places and put an if around the entire do loop in case no entries are returned. "for" is clearly the more elegant construct to use in this case. Sure, with FindFirstFile you can almost guarantee that at least one dir entry will be returned (i.e. ".") but other MS APIs use the same paradigm and "for" is correct for them too.
I do agree about the whole "using || as a conditional sequencing operator" crap though. Not only is it hard to read, it's also hard for the compiler to reason about since || is now a control structure as well as an arithmetic operator.
Admin
captcha: captcha
panic: stack overflow
Admin
>and put an if around the entire do loop in case no entries are returned.
Nah. My standard idiom is
Agree that this could have been made a little more appealing, though.
Not that anyone cares, but the ugliness is in Win32. The underlying OS function in NT is essentially based on functions 'open dir', 'read dir', and 'close dir'.
Admin
Ah, WTF?
My code doesn't normally look that awful.
And why can't I edit it?
Admin
> The ISO standard is actually YYYY-MM-DD, which is both easy to read, maintain sortability and unambigouness. What <i>really</i> bothers me is how it is > possible to come up with the MM/DD/YY date format idea in the first place.
MM/DD/YYYY is a WTF all on its own. Someone should do an article;-)
Admin
I was being sarcastic - I'm with Hank also. :)
- Tim
Admin
I agree with Anon... compare the STL iterator
for (SessionMap::iterator j = m_sessions.begin(); j != m_sessions.end(); ++j)
{
//do something with *j
}
or the Design Patterns iterator:
CIDIterator i = p->GetIDIterator();
for (i.First(); ! i.IsDone(); i.Next())
{
// do something with i.CurrentItem();
}
To the do while loop:
HANDLE hFile = FindFirstFile(sPath, &findFileData);
if (hFile != INVALID_HANDLE_VALUE)
{
do
{
// do something with findFileData
} while (FindNextFile(hFile, &findFileData));
// Close the "iterator"
FindClose(hFile);
}
The MS version fills the view with scaffolding, while the others go about their business in a line or two
Admin
Saying November first sets up a context to place the fourteenth, because "fourteenth" doesnt mean anything without knowing the month. By using that thought process, we should also be saying "2006 November fourteenth", but usually we dont need specify the year in conversation, so we tack it on the end after a comma when we need to. "November 14th, 2006". Since this is what we are used to in conversation, it is why we write MM/DD/YYYY.
And "fourteenth of November" takes longer to say than "November fourteenth". We like to shorten our sentences as much as possible (ain't, y'all, wouldya, etc.)
Admin
This is exactly what scripting languages are for. And this is exactly the kind of code that people too dumb to recognize scripting languages will write. Wonderful!
Admin
Fourteenth actually does mean something. What day is it? The fourteenth. Simple answer if you know the month. The same is true for November fourteenth if you know the year. Thus no reason to place the month first. Also the way you pronounce it should not count as a reason to write it that way, eg: $50 (which is stupid too, it should be 50$)
Admin
That's not how many other countries say it. The MM/DD/YYYY convention is an American English convention. In other English-speaking countries (UK, Australia) they write "14 November, 2006." It really makes much more sense. But still, for computers, YYYYMMDD is the most logical and usable format. That way you have a simple sortable number or string.
Admin
The convention of putting the year last was started several hundred years before anyone thought of devising a mechanical process for sorting.
Admin
As the man said.... "Born on the 4th of July":P
And the Americans still get it wrong and argue about it! MM/DD/YYYY is a retarded concept anyway you think about it!
Admin
im not sure what it does either. ive been Dir'ing on both my windows and *ix boxes and see no real reason for that check.
Admin
oops i meant to quote:
Admin
The "if (tmpstr[1] == 'D')" catches the line in the header which looks like this:
Directory of C:\Documents and Settings\jamescu
Note that it attempt to grab the directory name by searching backward from the end (actually it starts a character after the end, but that's another WTF), looking for the last space, so in this case, it would consider the directory name to be just "Settings\jamescu"
The "if(tmpstr[2] == '/')" catches line in the body whch look like this:
11/01/2006 12:10 PM 88,791 Connection Speeds.pdf
Note same problem searching for spaces, but the problem that apparently Hank had set his date format to 2006/11/01 (which can be done easily by going to Control Panel and setting your region to "English (South African)"). Now there's not slash at tmpstr[2], and nothing is placed in the Files array.
Admin
I'll agree that FindFirst and FindNext file aren't *quite* as pretty as either of those, but I don't see what's wrong with fluffy's suggestion once you move the FindClose out of the loop:
HANDLE search;
for(search = FindFirstFile("dirpath", &data); search != INVALID_HANDLE_VALUE; FindNextFile(search, &data))
{
...
}
FindClose(search);
What's wrong with that? I've written far longer expressions inside if statements curtesy of the STL (I have a tendency to not typedef every type that I will be using an iterator from which has led in a few potentially WTF-worthy instances in having a type of the iterator that's as long as the whole initialization expression in this example), so while it is a little long it's not too bad. The only other wart is that search isn't local to the loop. However, I will take this point to mention that in the case of the Unix convention (opendir/readdir) it isn't either.
So it's not perfect, but to write it with first(); if { do {... } while(next()); } seems to be taking lemons and making... I dunno, some foul potion. :-p
(Now, I will admit to not actually having used these interfaces in practice, so it could be that the for version introduces some weird issue I'm not forseeing.)
Admin
I agree the code snippet presented was poorly designed. But I can't count how many times I've seen poor (even truly abysmal) design that works well enough to be useful. It's no surprise it failed; but then, quite a lot of code fails for some unforeseen reason eventually. This bit just had no legs to begin with.
Tim, was it your intention in creating the CodeSOD to draw out the WTF commentors?
What's most interesting to me is watching some of the commentors here beat their chests about how non-WTF they or their adopted platforms are. It's more amusing than watching monkeys flinging poo at each other. So thank you, Tim (& Alex) for that.
For all of you ranting about FindFirstFile/FindNextFile vs. STL and iterator patterns, and do vs. if loops, and MS vs. UNIX -- um, you have a little poo there, just under your ear.
Please keep up the column - I enjoy the challenge of figuring out what the code does and why it shows poor design.
->A
Admin
Oh, I almost forgot to include those arguing about cultural idioms (dates, money, America vs. The Rest of the World).
All of you, please keep championing your causes. You'll get your way in the end, because nobody can help but notice how right you are. All of you.
->A