• (cs)

    This is outstanding - the coder knows at least something about alignment and padding, but has never looked into the basic functions that the language provides.

    I always wonder - at some point during this person's quest to code this up, surely a synapse fired somewhere that said, "ya know...I'm doing something pretty common here...surely someone has done this before, and done it well?". Do these people have a hormone imbalance that suppresses these thoughts?

    Oh, and WTF.

  • (unregistered)

    I disagree that this is a WTF.  SHFileOperation does a hell of lot more that CopyFile.  It has a progress UI, UI for various errors, it allows the user to cancel the copy if it's taking a long time (slow network maybe), etc, etc.

    If there's a WTF here it's the complexity of the code needed to call it, not that fact that they're calling it in the first place.  But that's life when calling an API intended for a different language than the one you're using.

  • (cs)

    LOL.

    When does being '1337' get in the way of productivity? When you're manipulating things on a binary level in VB for no other purpose than to copy a simple file.

    Either that or the author of this code is so 1337 that he or she can't possibly fathom the concept of RTFM.. :)

  • (cs) in reply to
    :

    I disagree that this is a WTF.  SHFileOperation does a hell of lot more that CopyFile.  It has a progress UI, UI for various errors, it allows the user to cancel the copy if it's taking a long time (slow network maybe), etc, etc.

    If there's a WTF here it's the complexity of the code needed to call it, not that fact that they're calling it in the first place.  But that's life when calling an API intended for a different language than the one you're using.

    Hmm. Actually, you bring up a very valid point. For once one of the 'Anonymous Defender Squad' is right. And I agree with the real WTF here. VB has a very hard time calling some external API's not designed to be used with it, in making it an environment for quickly creating apps, they necessarily had to simplify things a bit... I suppose they could have come up with a  way to simplify coding against the Win32 API a bit, like some very basic support for certain types, and data packing/alignment.

  • (unregistered)

    I am a member of the 'Anonymous Defender Squad'.

    I was going to defend the code just like the other guy did, but he beat me to it.

    Anonymous Defenders...unite!

  • (unregistered)

    Ahahahahahbaaaabhahahaha

     

    I was thinking, man this code is nice, it looks like a lot of work went into it.  So naturally I figured that it was cut&pasted.  So I googled it and found:

     

    http://www.softcircuits.com/dev/vb_res/tip200106.shtml

     

     

    -Ron

  • (cs)

    I figured it was cut and pasted because I am teh sarmtest

  • (cs) in reply to

    Ron!!!! you're not supposed to reveal your NAME if you're anonymous!!!!

    Now everyone who posts anonymouse will be called "Ron"

  • (cs) in reply to Goudinov

    LOL anonymouse ... d'oh! no edit button

  • (cs)

    Nope, it's a WTF.

    He creates two new collections, adds exactly one element to each, tests if either is zero (which neither can be), and then loop as if either has several elements. Acknowledging that would reduce 15 lines to 2.

    Similarly, he sets some flags to contants at the start, and then tests what their values are.

    But the biggest WTF is the when he notes that his SHFILEOPSTRUCT struct is packed wrong, instead of fixing the definition, as any sensible programmer would do (eg, http://kandkconsulting.tripod.com/VB/Code/tips/recyclebin.htm) , he goes through this long, ugly, confusing rebuilding of it. 

    This is clearly the work of an assembly programmer forced to program in VB, who, instead of actually learning the language, just seeks out new ways to do battle with it.

  • (unregistered)

    I'm in no way defending the original code, but I've had to abandon the built in VB operations and drop to windows API many times -- most commonly because of file size limitations on reading/writing files (limited to fFileSize <= 2GB). I'm not sure if these limitations apply to FileCopy or FSO, but that may be a good, valid reason to *not* use 'em.

  • (cs) in reply to JamesCurran
    JamesCurran:

    Nope, it's a WTF.

    He creates two new collections, adds exactly one element to each, tests if either is zero (which neither can be), and then loop as if either has several elements. Acknowledging that would reduce 15 lines to 2.

    Similarly, he sets some flags to contants at the start, and then tests what their values are.

    But the biggest WTF is the when he notes that his SHFILEOPSTRUCT struct is packed wrong, instead of fixing the definition, as any sensible programmer would do (eg, http://kandkconsulting.tripod.com/VB/Code/tips/recyclebin.htm) , he goes through this long, ugly, confusing rebuilding of it. 

    This is clearly the work of an assembly programmer forced to program in VB, who, instead of actually learning the language, just seeks out new ways to do battle with it.



    Oh. Gee. haha. I knew I smelled a WTF in the whole structure diatribe.  The beauty of this code is something I didn't catch immediately (I only skim, sue me) That they are 1 element collections. How....efficient, Also someone mentioned that the code is cut-n-paste. Which does make it 100% pure WTF. The author of the 'routine' was probably a barely trained monkey.
  • (cs) in reply to

    I'll admit to having used FindFirstFile/FindNextFile in a VB6 app. The Dir() function isn't stable if you're deleting items out from underneath it. Possibly we should have captured the results then performed the deletes (which is essentially what FindFirstFile does).

    Anyway, when you start getting pain like this - it's often better to write a C++ DLL that wraps the complex API in a friendly(-ier) interface. Just be aware of how VB calls Declared functions. Strings are ALWAYS converted from the internal Unicode format to byte-oriented strings (char*), using the user's current code page. When the called function returns, VB converts the strings back the other way in case it was an in/out parameter, so be careful not to trash any input-only strings inside the DLL.

  • (cs) in reply to Mike R

    In ways, I wish I could edit posts... Gah

    I used to work with someone like this...

    Their code was about 85% copy-paste, with a stream of wtfs inbetween the pasted bits. Most of what they worked on was rewritten.  Half the time it didn't work.

    The author of this gem gets extra points for

    1. Using code he found, which he did not understand.
    2. Not removing the collection requirement from the code, evidence of the lack of understanding
    3. Not finding a simple soltion (almost similarly named to the function they wrote) by taking the requisite 5 minutes and searching the documentation for "copy file"

  • (cs) in reply to Mike Dimmick

    This reminds me of an application I saw that was moving a file from a local directory to an ftp site on the network.  It was very funny because this application had a series of interesting variables.

    Dim m_FtpLocation as String
    Dim m_ServerName as String
    Dim m_DriveLetter as String

    m_FtpLocation = "ftp://ServerName"
    m_ServerName = Replace(m_FtpLocation, "ftp://")
    m_DriveLetter = "Z"

    ' then a bunch of code to map a drive to that server, and copy the files that way over a series of a crap load of copy/paste code.

    The reason it reminds me of it is the abundance of Constants and class variables that just refer to themselves without any conditions.  Seems a bit unnecessary if you ask me.  Anyways.  I hope I never have to touch vb ever again.

  • (cs) in reply to Mike Dimmick
    Mike Dimmick:

    I'll admit to having used FindFirstFile/FindNextFile in a VB6 app. The Dir() function isn't stable if you're deleting items out from underneath it. Possibly we should have captured the results then performed the deletes (which is essentially what FindFirstFile does).

    Anyway, when you start getting pain like this - it's often better to write a C++ DLL that wraps the complex API in a friendly(-ier) interface. Just be aware of how VB calls Declared functions. Strings are ALWAYS converted from the internal Unicode format to byte-oriented strings (char*), using the user's current code page. When the called function returns, VB converts the strings back the other way in case it was an in/out parameter, so be careful not to trash any input-only strings inside the DLL.



    Or just write a small COM compnent, which uses BSTRs to get around the string conversion issue.
  • (unregistered)

    Hahahahahaha - I can not even defend this code!  Although sometimes it is easier to reinvent the wheel, this one is too much.  (I wrote my own file copy dll that copies the files into a byte array and then transmitts them to the destination file in chunks with pauses in it.  The reason I did this was to control how much data is buffered and passed over the network.  Running on 384k circuts makes things SLOW (chews up all of the circut) when you copy large files via explorer.)

  • (cs) in reply to Mike R

    Yup, this is the classic move of a WTF programmer.  These are people who know the fundamentals of the language but not the architecture of the environment itself, and don't know the library and don't bother to look.  They violate the first rule I try to teach new programmers:  "You should be writing business logic (place an order) not techical actions (sort a list, copy a file).  If you find yourself writing a technical action, there is a library call you haven't found yet."

    Here's a classic example:  I ran across hundreds of lines of Java code across multiple objects that passed an object from a Servlet to a JSP.  It could have been replaced with a single call to request.setAttribute().  This person however seemed to think that JSPs were executed on the browser, so they came up with this crazy system that serialized to a local file on the client from the servlet and deserialized off the client again in the JSP.

  • (unregistered)

    Woah. A VB scripter (I can't stoop low enough to call them "coders" or even "programmers") that knows win32api.

    Perhaps an ex-asm or C coder that's been forced to code VB for bananas.

  • (unregistered)

    Heh, I think this guy was getting paid by the hour or something.

  • (cs) in reply to

    <font face="Georgia">Bat's Rule of Theft

    Theft is a virtue.  Originality is a vice.


    Which is to say: if you're writing something from scratch, you're wasting time.  The age and complexity of a function in production code is directly proportional to the number of bugs that have been caught and fixed.  If you're going to rip it out and replace it with something newer and simpler, be aware that you're throwing away a lot of effort and work.  Granted this is sometimes a good thing, but it's not always the case.  Look at how long it took the Mozilla project to produce something that wasn't worse than Netscape 4.

    In the present case, this applies in reverse: the "old and complex" function is what's built in to the library.  The "new and simple" version is what the WTF programmer was trying to write.  He let it build up in complexity, of couse, because he's an idiot, but if he'd been wise enough to follow the Rule of Theft, he'd have realised that even Visual Basic has code to do what he was trying to do, and it would be able to handle cases he didn't think of.

    But I guess you knew that already.

    </font>

  • (cs)

    Several folks have suggested the theory that this is a 'C' programmer who has been forced to use VB. I can't believe that's the case. A 'C' programmer would have gone ahead, packaged SHFileOperation in a DLL, and called the function in the DLL. He/she also would not have done nearly this inept a job at calling SHFileOperation directly.

  • (cs) in reply to GWheeler

    I think this is just someone who found a piece of code somewhere that could copy multiple files, which had all kinds of neat additional features and thought, "Wow, cool! I need that code."

    And then of course, since he didn't need all the extra functionality and only needed to copy one single file, he just turned it into a real WTF. This code shows the brilliance of someone who knows his way in the Windows API, combined with the stupidity of an inexperienced VB programmer. So basically, it's the work of a smart person destroyed by the stupid person who took it over... [;)]

    Just look at the code! Everything from the line:

    'Parent window of dialog box--just use 0
    is pure brilliance of trying to get a nice Windows API working with dozens of options. And the crap above it is just added so it picks some standard settings and only copies one file...

  • (unregistered)

    http://www.softcircuits.com/dev/vb_res/tip200106.shtml

  • (unregistered) in reply to

    Dollars to donuts it's a C programmer being forced to use VB.

  • (cs) in reply to Katja

    re what Katja said, I've often found that the Win32API's... veritable smorgasboard of options causes more trouble than good. When you need a half-dozen arg function, at least one of the args being a half-dozen-part struct, to do anything, there's something wrong.

    Which is why I'm now going to shamelessly plug Tk. Compare:

    HWND CreateWindow(LPCTSTR lpClassName, LPCTSTR lpWindowName, DWORD dwStyle, int x, int y,
    	int nWidth, int nHeight, HWND hWndParent, HMENU hMenu, HINSTANCE hInstance, LPVOID lpParam);

    to

    Tk_Window Tk_CreateWindow(Tcl_Interp * interp, Tk_Window parent, CONST char * name, CONST char * topLevScreen);
    Tk_MoveResizeWindow(Tk_Window tkwin, int x, int y, int width, int height);

    IMO, splitting the args between two or more functions adds a huge amount to the clarity. I don't have to individually comment each of the args to Tk_MoveResizeWindow()  every time I call it, because they're obvious. The same applies for Tk_CreateWindow, as there's a straightforward standard for Tk function calls (interpreter, subject, other stuff, in that order).

    Who says you always want to position a window the moment you create it, anyway? If you don't, you end up with a bunch of arbitrary numbers in your call to CreateWindow(), to befuddle whoever next comes upon your code.

  • (unregistered)

    Ah tk and similar libraries. What better way to create an app than to use some crazy wrapper or graphics function library that provides all the win32 functions in a slightly different naming format. I never understand people who use wrappers, and who can't just get on with things.

    Now excuse me, I'm off to finish my MFC app.

  • (unregistered)

    There's a good reason not to use the FileSystemObject... it isn't necessarily there on the target machine.  I believe the FSO shipped with 2K/XP but not 9x (not sure about ME or NT).  If those users haven't installed the last couple versions of IE they won't have the FSO.  We all know how crappy 9x/ME are but unfortunately a lot of businesses still use these older OSs and there are a lot of "grandma" type users using them. 

    As others have noted, the API functions allow for more functionality than CopyFile or FSO.

    Of course, that doesn't excuse that incredibly convoluted function.  I'm defending the use of API calls to to filecopies in certain situations, but not this dude's function in particular.  :)

  • (cs) in reply to Katja

    I basically agree with Katja.

    Also, there are times when you do not want to use FSO (or so they told me here at work). Not sure why, maybe there are incompatibilities in the win95, 98, nt, 98se, 2000, xp and 2003 versions?

    Drak

  • (cs) in reply to Drak

    Argh, I totally missed page 2... Should have looked better. Sorry for repeating what mister anonymous (ron???) said.

    Drak

  • (unregistered)

    Whomever did this has WAY too much time on thier hands.


  • (cs)

    <FONT color=#000000>Just like us...[8-|]</FONT>

  • (cs) in reply to
    :

    Now excuse me, I'm off to finish my MFC app.

    Funny stuff!
  • (unregistered) in reply to
    :
    Ah tk and similar libraries. What better way to create an app than to use some crazy wrapper or graphics function library that provides all the win32 functions in a slightly different naming format. I never understand people who use wrappers, and who can't just get on with things.


    There's a couple of reasons, in my case.

    Looking at that Win32 example I gave, there's this type LPCTSTR. Which I think they introduced just in case the type meaning "a 32-or-more-bit pointer to a character string" ever changes. And I haven't the foggiest what the "T" stands for. 'Nuff said, really.

    More importantly, if you write your program with the Win32 API, it will only work on Windows. If you write it with Tcl/Tk, GTK+, or whatever, it will (theoretically) work on any OS. This is the most major point of wrappers for OS-specific functions.

    (Incidentally, I find your rather dismissive phrasing, and lack of anything to back your opinion, rather trollish.)
  • (cs) in reply to

    :

    Looking at that Win32 example I gave, there's this type LPCTSTR. Which I think they introduced just in case the type meaning "a 32-or-more-bit pointer to a character string" ever changes. And I haven't the foggiest what the "T" stands for. 'Nuff said, really.

    FYI, the LPCTSTR type changes what it points to depending on whether UNICODE is defined or not.  If UNICODE is defined, LPCTSTR is actually LPCWSTR (pointer to null terminated 16 bit character string).  If not, then it's a LPCSTR (pointer to null terminated 8 bit character string).  It's there to allow for platform compatibility between the 9x and NT based Windows OSs (NT based use Unicode natively).

  • (cs)

    If I'm not mistaken [;)], this forum software was first written by a summer intern at MS on some of the earliest asp.net bits.

    There was a little brouhaha over the Telligent aquisition of the rights to it.

    I think that it survives because the people who implement it and view forums using it are mostly technical people who are mostly interested in the content.

    I worry sometime when I see mention of the fact that the new provider model has some relationship to the first implementation, but I try to focus on the fact that it is just the idea and not the code.

    I'm debugging a truly horrrible integration/extension of PopForums, which also seems to have be coded before best practices existed; never finally closing connections. That all pales in comparison to the VB scripter's extensions in c#. He has three hours to write an email about not fixing his crap but doesn't have 5 minutes too fix it.

     

  • (unregistered) in reply to brandonh6k
    brandonh6k:
    FYI, the LPCTSTR type changes what it points to depending on whether UNICODE is defined or not.  If UNICODE is defined, LPCTSTR is actually LPCWSTR (pointer to null terminated 16 bit character string).  If not, then it's a LPCSTR (pointer to null terminated 8 bit character string).  It's there to allow for platform compatibility between the 9x and NT based Windows OSs (NT based use Unicode natively).


    Ok, I mucked up the LPCTSTR definition. I think that works for my argument though, not against it. After all, if a biased-against-win32 person like myself can't work out their type names, who can? *g*

    From what you've got there, NATIVE_STR would be a better name, IMO. You don't need the LP, 'cos all Ps are L nowadays, and all strings in C are really pointers. If C means character, you don't need it, 'cos it's a string, fer crying out loud, of course it's made of Cs. And I still don't know what the T means.

  • (unregistered) in reply to

    hmm... for some reason, it won't let me log in. that last post was me.

  • (cs) in reply to

    :
    hmm... for some reason, it won't let me log in. that last post was me.

    Er..um... And we still don't know who you are......

  • (unregistered)

    gak. Still not working.

    My normal nick is Irrelevant. Dunno what's wrong with it.

  • JonathanWood (unregistered)

    Well, the code was derived (without credit, I might add) from code that *I* wrote. So I thought I'd add my 2 cents.

    First of all, SHFileOperation does more than simply copy a file. It can delete, copy, move, etc., even recursively through subdirectories, with many options. In addition, it display the same progress dialog box used by the system to perform these operations. When I need the functionality provided by FileCopy, I use FileCopy. For all those who think FileCopy makes SHFileOperation useless, all I can say is WHOOSH! (It went over your head.)

    Next, regarding some of the criticisms of the efficiency of the routine. The original code was implemented as a VB class, which had properties you could set first before calling methods to perform the specified tasks. The code posted had been modified to be a single routine, which made some of the extra variables unecessary. You can see the original class at http://www.softcircuits.com/sw_vbsrc.htm.

    As far as JamesCurran's comment that there are easier ways to pack the structure. All I can say is he is wrong. It's very easy to criticize code you don't understand. Try testing it fully, as I have.

    As far as the languages I use, I am very proficient in C/C++, Visual Basic, assembler and others. Not being tied to a single language is hardly a limitation to my way of thinking. And I never had understood the mentality that VB programmers should only use existing routines and not code anything in depth.

    Jonathan Wood
    SoftCircuits

  • Anonymous (unregistered) in reply to

    LPCTSTR means:

    Long
    Pointer to a
    Constant
    TCHAR
    STRing

    TCHAR being defined as char or wchar_t depending on if UNICODE is defined or not

    so basically LPCTSTR = const TCHAR*
    there's a LPTSTR as well, and all the other combinations you can think of (LPSTR, LPCSTR, LPWSTR, LPCWSTR, LPTSTR, LPCTSTR)

    When you know what the letters mean, it becomes quite easy to figure out what the type names mean.

  • (cs) in reply to JonathanWood
    JonathanWood:
    Well, the code was derived (without credit, I might add) from code that *I* wrote. So I thought I'd add my 2 cents. First of all, SHFileOperation does more than simply copy a file. It can delete, copy, move, etc., even recursively through subdirectories, with many options. In addition, it display the same progress dialog box used by the system to perform these operations. When I need the functionality provided by FileCopy, I use FileCopy. For all those who think FileCopy makes SHFileOperation useless, all I can say is WHOOSH! (It went over your head.) Next, regarding some of the criticisms of the efficiency of the routine. The original code was implemented as a VB class, which had properties you could set first before calling methods to perform the specified tasks. The code posted had been modified to be a single routine, which made some of the extra variables unecessary. You can see the original class at (snipped to enable posting). As far as JamesCurran's comment that there are easier ways to pack the structure. All I can say is he is wrong. It's very easy to criticize code you don't understand. Try testing it fully, as I have. As far as the languages I use, I am very proficient in C/C++, Visual Basic, assembler and others. Not being tied to a single language is hardly a limitation to my way of thinking. And I never had understood the mentality that VB programmers should only use existing routines and not code anything in depth. Jonathan WoodSoftCircuits

    Well said. The view that programmers (of any language) should only use existing routines and not code anything in depth underlies most of the comments on TDWTF.

Leave a comment on “Basic Complexity”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article