• Psy (unregistered)

    Wow. Just... Wow

  • Dwayne (unregistered)

    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."

  • (cs)

    Sounds like a bad case of shell programmer's disease.

  • mAize (unregistered)

    My C++ is rusty, but I'll take a stab at it:

    • Code could blow up literally anywhere
    • Big time Null terminator issues
    • Err, isspace()?
    • Assuming file/directory lengths are < 200
    • No argv[1] existance testing
    • File[] seems to be a 1d array rather than 2d
    • What is "Files[i][0] = 0;" for????
  • mAize (unregistered)

    Also,

    	printf("Too Many Files to Process!   Please select fewer files!\n");
    If anything, it should read 'Choose a Directory with less files'. Don't think anyone's selecting any kind of file...  

     

  • (cs) in reply to mAize
    Anonymous:
    • What is "Files[i][0] = 0;" for????

    "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.

  • Brendan (unregistered)

    Tim Gallagher:
    Hank is one of those weird people who thinks that dates should be YYYYMMDD format.

    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) .

  • (cs) in reply to Brendan

    dirent is a Unixism. There is no standard way to do directory operations in C or C++.

  • (cs) in reply to Goplat

    Goplat:
    dirent is a Unixism. There is no standard way to do directory operations in C or C++.

     This is true (for now), but there are heavily portable ways to do it.

     

    Dwayne:
    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

    Huh?
     

  • Tim (unregistered) in reply to EvanED
    EvanED:

     

    Dwayne:
    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

    Huh?
     

     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.
     

  • ajk (unregistered)

    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.

     

  • DarthMoo (unregistered) in reply to ajk

    Uh, system() is a blocking call. Get back to stacking those Elmo dolls.

  • Jon Haugsand (unregistered) in reply to Brendan
    Anonymous:

    Tim Gallagher:
    Hank is one of those weird people who thinks that dates should be YYYYMMDD format.

    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) .

     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

     

  • Jens (unregistered)

    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... :-) 

  • DanielJC (unregistered) in reply to Dwayne
    Anonymous:

    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

    Are you actually being serious? If so, your programming knowledge let alone API knowledge is severely deficient!

  • (cs) in reply to Jens
    Anonymous:

    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... :-) 

    Heh likewise on a french Windows:
    Z:\>Dir
     Le volume dans le lecteur Z s'appelle web
     Le numéro de série du volume est 1742-0135
    

    Répertoire de Z:</pre>

  • (cs) in reply to Jon Haugsand
    Anonymous:

    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.

    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.

  • (cs)

    I haven't tried to analyze the code, but ofcourse the REAL WTF here is:

    ...the custom compiler they wrote for their in-house language...

    I wonder why the job they had to do was so special that it required a custom programming language. 

  • Oli (unregistered)

    Tim Gallagher:
    Hank is one of those weird people who thinks that dates should be YYYYMMDD format

     

    What's wrong with that format? It allows easy sorting of files by name in a directory system for one...

     

    CAPTCHA: giggity

  • (cs) in reply to ajk
    Anonymous:

    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.

     

    std::system() is blocking, otherwise how would it return the executed command return status ?
     

  • (cs) in reply to Jens
    Anonymous:

    It took me quite a while to figure out, what "tmpstr[1] == 'D'" was supposed to do... :-) 

    Well enlighten me, because I'm still in the dark here... Is it to test for a directory?

  • (cs)

    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. 
     

  • Jimbob (unregistered) in reply to mAize

    > 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

  • MHolmgren (unregistered) in reply to Jimbob

    Oh yeah, I guess you are one of those guys who think time should be written on the format mm/ss/hh? :-)

  • (cs)

    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 ???

  • Brenda (unregistered) in reply to AstorLights

    AstorLights:
    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 ???

    There are more poblems in that peice of code then in M$ windows(and we all know how many poblems there are in windows).

  • fluffy777 (unregistered) in reply to Tim
    Anonymous:
    EvanED:

     

    Dwayne:
    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

    Huh?
     

     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.
     


    how about:

    for(HANDLE search = FindFirstFile("dirpath", &data);
        search != INVALID_HANDLE_VALUE;
        FindNextFile(search, &data) || (FindClose(search), search = INVALID_HANDLE_VALUE)) {
     ...
    }
  • fluffy777 (unregistered) in reply to Tim
    Anonymous:
    EvanED:

     

    Dwayne:
    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

    Huh?
     

     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.
     


    how about:

    for(HANDLE search = FindFirstFile("dirpath", &data);
        search != INVALID_HANDLE_VALUE;
        FindNextFile(search, &amp;data) || (FindClose(search), search = INVALID_HANDLE_VALUE))
    {
       ...
    }

    Sorry, tried to encapulate in [ code ]  [ / code ] and it screwed up (I'm running safari so preview doesn't work, and if you try it and go back the editor is gone, WTF?)
  • _js_ (unregistered) in reply to fluffy777

    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.

  • Tim (unregistered) in reply to fluffy777
    Anonymous:
    how about:

    for(HANDLE search = FindFirstFile("dirpath", &data);
        search != INVALID_HANDLE_VALUE;
        FindNextFile(search, &amp;data) || (FindClose(search), search = INVALID_HANDLE_VALUE))
    {
       ...
    }

    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.

  • maffiou (unregistered)

    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...    

  • Eddie Pedant (unregistered) in reply to Tim
    Anonymous:

    Use a do-while loop as mentioned above - it's what they're for, ffs.

    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.

  • lynne truss (unregistered) in reply to mAize
    >If anything, it should read 'Choose a Directory with less files'. Don't think anyone's selecting any kind of file... 
     
    Well, at least he got the grammar right. 

     

     captcha: captcha

     panic: stack overflow

  • dave (unregistered) in reply to Eddie Pedant

    >and put an if around the entire do loop in case no entries are returned.

     Nah.  My standard idiom is

     

       HANDLE h = FindFirst(....);

       bool ok = (h != INVALID_HANDLE_VALUE);

       while (ok) {

            .... process....

           ok = FindNext(....);

       } ;

     

     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'.

     

  • dave (unregistered) in reply to dave

    Ah, WTF?

     My code doesn't normally look that awful.

    And why can't I edit it? 

  • PLasmab (unregistered) in reply to Jon Haugsand

    >  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;-)

     

     

  • (cs) in reply to Brendan
    Anonymous:

    Tim Gallagher:
    Hank is one of those weird people who thinks that dates should be YYYYMMDD format.

    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.

    I was being sarcastic - I'm with Hank also. :)

    - Tim 

  • blaaaaaaaaaaaaa (unregistered) in reply to DanielJC
    Anonymous:
    Anonymous:

    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

    Are you actually being serious? If so, your programming knowledge let alone API knowledge is severely deficient!

     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 

  • Daniel (unregistered) in reply to MET
    MET:
    Anonymous:

    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.

    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.

    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.)

  • (cs) in reply to blaaaaaaaaaaaaa

    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!

  • Jurgen (unregistered) in reply to Daniel

    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$)

  • (cs) in reply to Daniel
    Anonymous:
    MET:
    Anonymous:

    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.

    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.

    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.)

    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.

  • (cs) in reply to Jon Haugsand

    Anonymous:
    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.

    The convention of putting the year last was started several hundred years before anyone thought of devising a mechanical process for sorting.

  • PLasmab (unregistered) in reply to Daniel

    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!
     

  • segmentation fault (unregistered) in reply to zamies

    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.

  • segmentation fault (unregistered) in reply to segmentation fault

    Anonymous:
    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.

     

    oops i meant to quote:

     

    zamies:
    Anonymous:

    It took me quite a while to figure out, what "tmpstr[1] == 'D'" was supposed to do... :-) 

    Well enlighten me, because I'm still in the dark here... Is it to test for a directory?

     

  • (cs) in reply to zamies
    zamies:
    Anonymous:

    It took me quite a while to figure out, what "tmpstr[1] == 'D'" was supposed to do... :-) 

    Well enlighten me, because I'm still in the dark here... Is it to test for a directory?

    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.

     

     

     

  • (cs) in reply to blaaaaaaaaaaaaa
    Anonymous:

     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 

    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, &amp;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.)
     

  • AlanM (unregistered)

    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

  • AlanM (unregistered) in reply to AlanM

    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

Leave a comment on “Please Select Fewer Files”

Log In or post as a guest

Replying to comment #:

« Return to Article