• LCrawford (unregistered)

    The frist thing I spotted as missing is a check for failure near end: if (retval == null) return FILE_NOT_FOUND; Which is a pre-built bitmap containing the words "File not found".

  • Niels (unregistered) in reply to LCrawford

    @lCrawford That wouldn't even be the worst thing to return.

  • MaxiTB (unregistered)

    This whole code stinks after memory leak because, spoiler, Bitmap is also an IDisposable and must be correctly freed :-)

  • (nodebb)
    // TODO implement this comment!!!
    
  • public static (unregistered)

    think about different threads calling that method.

  • (nodebb)

    They should have named it "ActivateTheFlashingLightsSequence" and the code should read a row from a database which doesn't exist.

    I mean, it's not like it's called from anywhere, so what does it matter how it's named or what it does.

  • (nodebb)

    On the scale of "TODO" comments, it's not great, but I just tripped over one in my company's codebase that is probably worse...

        // TODO
    

    That's it, just the TODO. The next line was actual code, so I have no idea what remains to be done.

    We have a "two code review acceptances before shipping" policy, and one of the reviewers who approved the commit that had this in it was ...

    ... (embarrassed silence) ...

    Steve_The_Cynic (oops).

  • hvd (unregistered) in reply to MaxiTB

    The bitmap is returned to the caller; the fact that it is not disposed of is not a WTF but exactly the right thing to do. It is the caller's responsibility to call Dispose() after it is done with it.

  • Stella (unregistered) in reply to MaxiTB

    This whole code stinks after brain leak because, spoiler, Programmer is also a Disposable and must be correctly freed :-)

  • (nodebb)

    Far too many "public" in 99.99% of all code bases....

  • mihi (unregistered)
    Comment held for moderation.
  • Fizzlecist (unregistered)

    At least it wasn't called from anywhere, so they did one thing right

  • Your Name (unregistered)

    I think there's one more WTF; when initializing a Bitmap with a Stream object you need to keep that stream open for the lifetime of the Bitmap.

    The metod returns a Bitmap that throws ObjectDisposedException on most (all?) method calls.

  • Tea Lovers (unregistered)
    Comment held for moderation.
  • (nodebb) in reply to TheCPUWizard

    Few things are as annoying as a needlessly sealed class or an internal method in a nuget library.

  • felix (unregistered)
    Comment held for moderation.
  • felix (unregistered)
    Comment held for moderation.
  • felix (unregistered) in reply to Your Name
    Comment held for moderation.
  • Tinkle (unregistered)

    I suspect this is an attempted work-around for the fact that opening a bitmap using the filename in the Bitmap constructor will lock the file for writes, so you can not save changes.

    @Your Name It probably works better in debug mode, as the garbage collector is a bit less aggressive.

  • Times OF National (unregistered) in reply to LCrawford
    Comment held for moderation.
  • Craig (unregistered)

    My thought as to the "why" was for handling an image in resources, as I believe you get that back as a byte array.

  • ascrack (unregistered)
    Comment held for moderation.
  • (nodebb)

    It's great to see that the body of code out there in the wild is so much better than it used to be, as demonstrated by recent bunch of What The Meh. It's dead code, delete it and move on.

Leave a comment on “Bitmaps and Streams”

Log In or post as a guest

Replying to comment #579579:

« Return to Article