- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- 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
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".
Admin
@lCrawford That wouldn't even be the worst thing to return.
Admin
This whole code stinks after memory leak because, spoiler, Bitmap is also an IDisposable and must be correctly freed :-)
Admin
Admin
think about different threads calling that method.
Admin
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.
Admin
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...
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).
Admin
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.
Admin
This whole code stinks after brain leak because, spoiler, Programmer is also a Disposable and must be correctly freed :-)
Admin
Far too many "public" in 99.99% of all code bases....
Admin
At least it wasn't called from anywhere, so they did one thing right
Admin
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.
Admin
Few things are as annoying as a needlessly sealed class or an internal method in a nuget library.
Admin
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.
Admin
My thought as to the "why" was for handling an image in resources, as I believe you get that back as a byte array.
Admin
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.