• LZ79LRU (unregistered)

    But why?

  • (nodebb)

    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:

    • if the code runs on a machine configured for European number formats, the comma isn't the "thousands" separator, but the decimal place marker (and by coincidence it will do approximately the right thing).
    • if the code runs on a machine configured for Indian number formats, the comma won't be where the code expects it (India uses ##,##,##,##,### formatting).
  • (nodebb)

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

  • (nodebb) in reply to Steve_The_Cynic

    applause +1 (can we have a button for that?)

  • (nodebb)

    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.

  • (nodebb)

    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.

  • LZ79LRU (unregistered) in reply to AGlezB

    Look at the bright side. You lost more brain cells reading this code than the person who wrote it had to begin with.

  • (nodebb)

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

  • Rob (unregistered)

    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?

  • Duston (unregistered) in reply to Mr. TA

    But then real professionals would put the threshold amount into an XML config file somewhere and then misname the variable that holds that value.

  • Molhanec, Michal (github)

    Bonus points for making it work with both locales which uses dot and which uses comma as a decimal numbers separator...

  • Molhanec, Michal (github)

    Bonus points for making it work with both locales which uses dot and which uses comma as a decimal numbers separator...

  • giammin (unregistered)

    seriously, this can't be real

  • retsep (unregistered)

    Was AI involved when generating the source code?

  • Sou Eu (unregistered)

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

  • (nodebb) in reply to Sou Eu

    This number is floored and mod 1000

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

  • MRAB (unregistered) in reply to retsep

    I can't tell whether AI was involved, but I can tell that RI (Real Intelligence) wasn't.

  • (nodebb)

    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.

  • Erk (unregistered)

    If you have an estimate you MUST use that amount of hours or more?

    I know, I'm giving them undue recognition here.

    I think it kind of did what it was supposed to do and, in my experience, that seems to be some kind of gold standard for systems development these days.

  • (nodebb) in reply to Rob

    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.

  • Gearhead (unregistered)

    if the code runs on a machine configured for Indian number formats, the comma won't be where the code expects it (India uses ##,##,##,##,### formatting).

    The format string is "{0:##.##}." The "." format specifier converts to either a comma or period but there is no "N" or "," specifier to create separators. The string munging is horrible, but would seem to work as intended. And what was intended is even more horrible. Why not delete all the files, or when they're no longer needed, or by file age?

    size = String.Format("{0:##.##}", byteCount / 1024.0);// + " KB";

    Ahem, KiB, GiB, ... </pendant>

  • Officer Johnny Holzkopf (unregistered) in reply to Balor64

    Even better, you can now account for fractions of bytes. Might save space, erm bits, parts of bits...

  • xtal256 (unregistered) in reply to retsep

    AI would not write code this bad! (go ahead, check with ChatGPT)

  • (nodebb) in reply to Rob

    Think of the nibbles!

  • MaxiTB (unregistered) in reply to Balor64

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

  • (nodebb) in reply to giammin

    seriously, this can't be real

    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.

  • (nodebb)

    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.

  • Jonathan (unregistered)

    All that string manipulation just calculates

    pow(1024.0, frac( log(size)/log(1024.0) ))

    Of course the result is used nonsensically, given the context, but at least calculate your nonsense efficiently!

  • (nodebb) in reply to CodeJunkie

    You don't even need to check the amount of free space at all....just delete the files.

    NO.

    If the files are temporary as in "store data during work, can be deleted when done", they should already be deleted. If this automatic cleanup fails sometimes (and lets be realistic, it does happen), or as a safeguard against files never cleaned up due to an omission in programming, you could delete a subset of the files; Either you'd have to implement a check, whether the file is still in use, or make assumptions, about how old files can be before they are certain to not be in use anymore. Both methods will have problems with the "drive is full" scenario though, but if the files are still needed, there's no way around it.

    If the files are temporary as in "caching expensive calculation", then you want to delete them as late as possible, for which the "delete when reaching storage threshold" idea is a good solution.

    "Just delete the files" never is.

    In reply to retsep:

    Was AI involved when generating the source code?

    I tried ChatGPT. It produced better code than this. Not copy-paste useful (that would have required more iterations of feedback to the bot, at which point I can just take the rough template and implement the logic myself instead of code-reviewing the AI), but at least code that was useful to learn methods of doing it correctly.

Leave a comment on “Free From Space”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article