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

[Ravindra]

Hello Duncan,
Thanks for your time. I have incorporated all of your comments.

Regards
Ravindra

PS: Read below my minor reasons.

On Fri, 2004-02-06 at 05:27, Duncan Mak wrote:
> 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.
>     }
> }
Here we have an extra function call always, even if we have a status of
OK. Current implementation does not do that.

> 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 took this idea from System.IO/MonoIO.cs.GetException (String, error).
I thought we might need the message, in future, if we want to insert
e.g. fileName or propertyName in FileNotFoundException or
PropertyNotSupportedException messages.

> 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.
This is absolutely right. I was wrong.