views:

151

answers:

4

I am writing code that catches this OutOfMemoryException and throws a new, more intuitive exception:

/// ...
/// <exception cref="FormatException">The file does not have a valid image format.</exception>
public static Image OpenImage( string filename )
{
    try
    {
        return Image.FromFile( filename,  );
    }
    catch( OutOfMemoryException ex )
    {
        throw new FormatException( "The file does not have a valid image format.", ex );
    }
}

Is this code acceptable to its user, or is OutOfMemoryException intentionally being thrown for a particular reason?

+5  A: 

Well, it's a good example of how an exception doesn't always mean what it says. This particular case (OutOfMemoryException for an invalid file) dates back to .Net 1.0, which had a more limited set of exception types from which the programmers of this library could choose. I assume it hasn't been changed since then to maintain backwards compatibility (a.k.a. "throwing good money after bad").

To be fair, I think it was about the worst possible choice for exception type they could have made here. When you open a file, and it happens to be big, and you get an OutOfMemoryException, it's logical to assume that you're actually out of memory and go barking up the wrong tree for awhile (there's more than one question on StackOverflow about this).

MusiGenesis
+1  A: 

If it's because the file is invalid, it's probably just trying to guess how big a buffer it needs based on some bytes in what it thinks is the header. Document your intent clearly with a test and you should be fine.

A: 

It's a misleading exception. Microsoft says:

"This problem may occur when you use the Bitmap.FromFile method and one of the following conditions is true:

  • The image file is corrupted.
  • The image file is incomplete.

    Note You may experienter code hereence this problem if your application is trying to use the Bitmap.FromFile method on a file stream that is not finished writing to a file.

  • The image file does not have a valid image format or GDI+ does not support the pixel format of the file.
  • The program does not have permissions to access the image file.
  • The BackgroundImage propery is set directly from the Bitmap.FromFile method."

(Bitmap descends from Image)

Of course, it's also possible to get this exception when you try to load an image which is too big. So you need to consider that.

Charles
It's almost not even worth adding the link when it's the first hit on Google. :)
MusiGenesis
@MusiGenesis: one could argue that it's not worth answering the question at all. :) (So why did I? I don't know)
Charles
+6  A: 

No, it is history. GDI+ was written quite a while before .NET ever came around. The SDK wrapper for it was written in C++. To keep it compatible, it couldn't use exceptions. Error conditions were reported with error codes. That never scales well, GDI+ has only 20 error codes. That's not much for such a large chunk of code.

The Status::OutOfMemory error code got overloaded to mean different things. Sometimes it really does mean out of memory, it can't allocate enough space to store the bitmap bits. Sadly, don't know how that happened, an image file format problem is reported by the same error code. There is no dedicated error code that could more accurately describe it, I guess.

For completeness, these are the error codes:

enum Status
{
    Ok = 0,
    GenericError = 1,
    InvalidParameter = 2,
    OutOfMemory = 3,
    ObjectBusy = 4,
    InsufficientBuffer = 5,
    NotImplemented = 6,
    Win32Error = 7,
    WrongState = 8,
    Aborted = 9,
    FileNotFound = 10,
    ValueOverflow = 11,
    AccessDenied = 12,
    UnknownImageFormat = 13,
    FontFamilyNotFound = 14,
    FontStyleNotFound = 15,
    NotTrueTypeFont = 16,
    UnsupportedGdiplusVersion = 17,
    GdiplusNotInitialized = 18,
    PropertyNotFound = 19,
    PropertyNotSupported = 20,
#if (GDIPVER >= 0x0110)
    ProfileNotFound = 21,
#endif //(GDIPVER >= 0x0110)
};
Hans Passant
Almost *any* of these would have been a better choice than `OutOfMemory`, even `GenericError`.
MusiGenesis
Especially the `UnknownImageFormat` seems suitable for a format that cannot be understood.
0xA3