[Mono-devel-list] Status to Exception handling in GDI+

[Duncan Mak]

Hey Ravindra!

I see that you implemented Status code checking in the System.Drawing
code base. I noticed that you've been using the following pattern:

void someFunction ()
{
    Status s = GdipDoSomething ();

    if (s != Status.Ok)
        throw GetException (s);
    else 
        return aValue;
}

Exception GetException (Status s)
{
    // a case switch here.
}

I don't think this is necessary and it's a round-about way of doing the
check. I would much prefer to see:

void f ()
{
    Status s = GdipDooSmoething ();
    CheckStatus (s);
}

void CheckStatus (Status s)
{
    if (s == Status.Ok) return;
    else {
       // put case switch here and throw the Exception immediately.
    }
}

I also noticed that, in your current GetException implementation, you
have:

String message;

switch (status) {
    // TODO: Test and add more status code mappings here
    case Status.GenericError:
        message = String.Format ("Generic Error.");
        return new Exception (message);

    case ....:
        message = ....;
}

there's no need for that, instead say:

switch (status) {
    case Status.GenericError:
        throw new Exception ("Generic Error.");

    case FooBar:
        throw new FooBarException ("Foo Bar error");

    ....
}

I think it is excellent that we now have checking of error codes in C#
from our GDI+ implementation, but I think doing this way is more
manageable and efficient than the current implementation.

We should also move the CheckStatus / GetException method out of the
abstract Brush class (gdipFunction.cs is not a bad location), as
managing the mapping from GDI+ Status codes to .NET Exceptions is not an
operation that is relevant to the Brush class.

Duncan.