Wednesday, July 22, 2009

Object Orientation in .NET ( has anyone at Microsoft heard of it ? )

It's not uncommon to be working with code and want to call an event explicitly. In this case, classes derived from the EventArgs class, have an Empty property, which returns an empty instance, useful for doing exactly that, without having to construct an object we will not use. However, while it's possible to use the 'new' keyword to override a property, doing MyMethod(this, RoutedEventArgs.Empty); returns a compiler error, as the Empty propertly returns EventArgs, not RoutedEventArgs. In other words, while RoutedEventArgs in particular represents a class where you are very likely to not examine the event arguments, you still cannot use the property, it is worthless.

This is true of ALL derived classes of EventArgs, although I can see how, for example, a mouse related event args can be assumed to contain data you need ( although this is often not true, if I handle the mouse left button click event, I don't often care about anything passed in ) The only place I can think of where this is almost sure to not be the case, is keyboard events, where usually I want to know what key was pressed. It is for this reason that I've never tried to use KeyEventArgs.Empty, that is, assuming I were to forget that the construct as a whole is useless.

In the same way when I use Bitmap.FromFile in C# in general, I need to cast the Image that is returned, to a Bitmap. This is because it's calling a base method, I guess. That makes sense, but then why not make the base method return a Bitmap, given that you can't alter a method by return type, however, if I called Image.FromFile and got back a Bitmap, I could quite happily assign the value to an Image, and it would downcast. In fact, all things being equal, it's possible that the class always returns a Bitmap, but always specifies an Image as the return type.

Yes, I am right. The Image.FromFile ultimately calls this:

internal static Image CreateImageObject(IntPtr nativeImage)
{
int type = -1;
int status = SafeNativeMethods.Gdip.GdipGetImageType
(new HandleRef(null, nativeImage), out type);
if (status != 0)
{
throw SafeNativeMethods.Gdip.StatusException(status);
}

switch (((ImageTypeEnum) type))
{
case ImageTypeEnum.Bitmap:
return Bitmap.FromGDIplus(nativeImage);
case ImageTypeEnum.Metafile:
return Metafile.FromGDIplus(nativeImage);
}

throw new ArgumentException(SR.GetString("InvalidImage"));
}

As an aside, surely this code would be prettier if the exception came from a default case, instead of just sitting underneath like that. Do they have coding standards in Redmond ? In fairness, if my suggestion would compile at all, I am pretty sure that it didn't work in early versions at least, b/c the compiler could not work out that every case had a return value. So, perhaps this code has just not been looked at in a long time.

The return type here is an Image. Bitmap.FromGDIplus looks like this:

internal static Bitmap FromGDIplus(IntPtr handle)
{
Bitmap bitmap = new Bitmap();
bitmap.SetNativeImage(handle);
return bitmap;
}
So, in a nutshell, even if I am using an Image, I get sent a Bitmap, but if I am using a Bitmap, or an Image, either way, the object is downcast to an Image for no good reason that I can see, before I get it back. No memory is saved, all that happens, is another reason for a meaningless cast.

Bitmap bm = (Bitmap) Bitmap.FromFile(@"c:\Image.jpg");

Is that really the best we can do ?

6 comments:

  1. You have said, you are using Bitmap.FromFile. But I believe there is no FromFile method on Bitmap. Do you mean Image.FromFile?

    There may be many reasons why this method returns Image, rather than a Bitmap.

    1 - Even though it returns Bitmap most of the time, if you make it to return Bitmap rather than Image, the code won't compile as MetaFile can't be casted to Bitmap. MetaFile.FromGDiPlus returns a MetaFile, not a Bitmap.

    2 - For future extensibility. They can add more concrete Image types without breaking the method interface.

    Or am I missing something here?

    ReplyDelete
  2. BTW, don't you have a RSS for this blog?

    ReplyDelete
  3. Hi here.

    Given that Bitmap is derived from Image, there is plainly a Bitmap.FromFile. That's kind of my point. Image has a FromFile method, and it returns a Bitmap which has been downcast to an Image. Bitmap has a FromFile method, and it also returns a Bitmap, which has been downcast to an Image. Your point about possible future derived classes from Image is interesting, because, as I show, if they did do this ( and it's not likely they will, I admit ), it would break the existing code, precisely because the .NET framework returns a Bitmap object when you call Image.FromFile currently, but downcasts it to an Image. That was kind of my point. If I had a reason to want the smaller object, I could not create one from a file, and if I want a Bitmap, I always have to cast it myself, which results in quite ugly code.

    I am just using a blogging service, I am not sure if it offers RSS. I will check it and see.

    ReplyDelete
  4. Hi Christian,

    as I see it, xxx.FromFile returns a type that corresponds to the data source, i.e. a Bitmap for most image formats, and a MetaFile for some such as .WMF and .EMF; however it always downcasts to Image, since that is what Image.FromFile promissed.

    So whenever you upcast to Bitmap, you are gambling the source wasn't actually a MetaFile image (or whatever other formats they might come up with in the future).

    I think we need an attribute ("noninheritable"?) so FromFile is only available as a static method for Image, not for image derivatives since that is only confusing for everyone. I for one never call Bitmap.FromFile, I just write (Bitmap)Image.FromFile since (Bitmap)Bitmap.FromFile looks really silly.


    On the Events.Empty matter I fully agree, it is useless; either use new XxxEventArgs() which requires appropriate arguments, or, what I prefer, just pass null.

    This is what they could have done:
    - define EventArgsBase similar to the current EventArgs, however without Empty;
    - have all XxxEventArgs, including EventArgs itself, derive from EventArgsBase, and add an Empty property that returns an XxxEventArgs with "appropriate" default values, whatever that means (KeyEvent? MouseEvent?)
    However I'm happy with passing null.

    Regards,

    Luc

    ReplyDelete
  5. *grin* yes, (Bitmap)Bitmap.FromFile does look silly, but (Bitmap)Image.FromFile just looks more counterintuitive ( why would the base class method return an instance of the derived class ? ) That it does, is part of the problem IMO.

    I don't like to pass null, because once I start using that paradigm, I need to check my event args for null in my events, as I have introduced the possibility. Ultimately, the best solution IMO is to always have an event that could also be called by your own code, call a method that doesn't take any arguments that are not needed.

    ReplyDelete
  6. "Image" is an interface, not a class.

    If most of your code uses Image as the type, the code will work later for new Bitmap2, or some other type name for an image that come out later.

    Does the Icon class also derive from Image. It might, I forget.

    ReplyDelete