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

Leave a comment on “Bitmaps and Streams”

Log In or post as a guest

Replying to comment #:

« Return to Article