- 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
But why?
Admin
Yeah, this code is so wrong-headed for the job that I can't imagine how the developer is able to breathe, much less engage in higher brain-function like standing up.
And it's worse than that, in fact, because:
Admin
And the real functional problem isn't that it deletes temp files if the free space is 1 to 10 TB, but that it doesn't delete them if the free space is 11 to 999 bytes...
Admin
applause +1 (can we have a button for that?)
Admin
The hilarious part IMO is that the logic to get the free space is already there. Just compare that against a threshold. But no, they went with this brain-dead method of formatting to a "user-friendly" number, then operating on that string.
Admin
My brain is pouring out of my ears so I can't be sure, but if you found someone like Joey from Friends and told him "delete temp files if free space dropped below 10 percent" you might get something like this.
Admin
Look at the bright side. You lost more brain cells reading this code than the person who wrote it had to begin with.
Admin
This approach is sub-par. Real professionals use Graphics.DrawString to output the size, then random DrawLine calls to make it harder to see, then OCR API call to convert the value back to a number. Only then you can do what the article shows (the double conversion and ToString and Parse and all that).
Admin
Am I the only one that saw the futility in converting a long to string to double, when just assigning di.TotalFreeSpace to byteCount would have done the same? Which makes you wonder, why use a double for a byte count anyway? Did they really expect fractional bytes?
Admin
But then real professionals would put the threshold amount into an XML config file somewhere and then misname the variable that holds that value.
Admin
Bonus points for making it work with both locales which uses dot and which uses comma as a decimal numbers separator...
Admin
Bonus points for making it work with both locales which uses dot and which uses comma as a decimal numbers separator...
Admin
seriously, this can't be real
Admin
Was AI involved when generating the source code?
Admin
The summary is almost right.
The number of free bytes is converted into the number of bytes, KB, MB, or GB using binary capacity (dividing by 1024 at each level). We no longer know the scale. This number is floored and mod 1000. If this number is no larger than 10, then delete the temp files,
0-10 bytes: delete the temp files 1,000-1,010 bytes: delete the temp files 1-10 KB: delete the temp files 1,000-1,010 KB: delete the temp files 1-10 MB: delete the temp files 1,000-1,010 MB: delete the temp files 1-10 GB: delete the temp files 1,000-1,010 GB: delete the temp files 2,000-2,010 GB: delete the temp files 3,000-3,010 GB: delete the temp files ...
The correct approach would have a threshold defined (in number of bytes) - preferably in some configuration. If the number of free bytes on the drive (d1.TotalFreeSpace) is less than the threshold, then delete the temp files. It would be interesting to see if deleting the temp files really deletes them or just moves to a recycle bin (and if so, did the original code remember to empty the recycle bin?).
Admin
There's no mod involved, it simply divides to generate the second string(1), and then truncates the string to lose the first comma / dot and everything that follows, which is divide-and-floor, not floor-and-mod.
(1) The first is transient in the conversion to double. (It to-strings the return from disk-free, then to-doubles the string.)
Admin
If someone uses strings for business operations instead of the appropriate type... well.
There's a reason why ToString() often has an overload for CultureInfo; and spoiler, sadly in .net the default is not InvariantFormat (which is the American one instead of for example using ISO) but it is the thread local CultureInfo. I guess the original thought was, who will use strings for anything but presentations to the user - serialization is XML anyway and there's a solid class framework for that around. Well, there ya go, even .net did some very, very poor choices in the past and to this day a ton of code style rule sets throw out warnings if you don't use the CultureInfo overload for good reasons.
Admin
Clearly the double for a byte count is future-proofing for when drives have zettabytes of space and you can't store the value in a long anymore.
Admin
TotalFreeSpace returns an 64 bit integer and this will never change, so the convertion to long is completely pointless no matter which use case. If in some distant future a larger number is required then this property will throw an exception and a new property will be available which returns for example a 128 bit integer - this actually has happened when 64 bit length arrays where introduced in .net (LongLength).
Oh and yeah, I totally feel your sarcasm, just wanted to clarify how this situation would be most likely handled by the framework design team ;-)
Admin
Unfortunately that looks too much likely and real, based on what I've seen done by "expert consultants". The kind who only shine through their PowerPoint presentations of their perfect architecture designs, getting high praise from the C suits, and invariably end up sucking at actually producing any real code. There are legions of these.
Combine with the "Stack Overflow" coder -- the kind of person who hits SO with a poorly chosen query, blindly takes the first answer, and then crudely adapts it to the task. It's like taking a hammer to fit a screw through a bolt.
There's an overlap between both categories. Unfortunately.
If you had never to deal with the type, then consider yourself blessed.
Admin
Why not just update the process that creates to the temp files to delete them? Or, if that's not possible for whatever stupid reason. Just schedule a job to delete the temp files periodically? You don't even need to check the amount of free space at all....just delete the files.