views:

382

answers:

5

I believe the factory method design pattern is appropriate for what I'm trying to do, but I'm not sure how much responsibility (knowledge of subclasses it creates) to give it. The example of using the factory method pattern at Wikipedia describes the situation I'm in almost exactly:

public class ImageReaderFactory 
{
    public static ImageReader getImageReader( InputStream is ) 
    {
        int imageType = figureOutImageType( is );

        switch( imageType ) 
        {
            case ImageReaderFactory.GIF:
                return new GifReader( is );
            case ImageReaderFactory.JPEG:
                return new JpegReader( is );
            // etc.
        }
    }
}

My question is, what does the figureOutImageType function look like? In this specific example, I would assume that it checks a file header in the InputStream to determine which image format the data is in. I would like to know if the ImageReaderFactory itself knows how to parse file headers and determine if the file type is GIF, JPEG, etc, or if it calls a function inside each Reader class that lets it know what type of image it is. Something like this, maybe:

int figureOutImageType(InputStream is)
{
    if(GifReader.isGIF(is))
        return ImageReaderFactory.GIF;
    else if(JpegReader.isJPEG(is))
        return ImageReaderFactory.JPEG;
    // etc.
}

It seems like having the factory know how to parse images breaks encapsulation, and letting the subclasses decide which one should be created is part of the factory method design pattern. Yet, it also seems like the figureOutImageType function is just adding some redundant code, because why not just have each subclass perform its check on the InputStream in the getImageReader function and skip the switch case?

I haven't had any experience using factories before, and I was hoping to get some insight from some people who have used them in the past on the best way to handle this problem. Is it okay to have the factory know about the inner workings of its subclasses, or should they be responsible for letting the factory know which to create, and how do you organize it all?

Thanks!

+1  A: 

Factory should have some idea about choosing the actual object to create. For example, WebRequest.Create method in .NET, should be able to choose between the different protocol clients by checking the protocol part of the Uri. It doesn't need to parse the whole thing. Just the part required to distinguish which class is going to be responsible for it (in your example, it'll probably be just the file header).

Regarding your question about breaking encapsulation, not really... Most of the time, the factory is hardcoded and already knows about different types of classes and their features. It already depends on the functionality offered by a known set of classes, so you are not adding much to it. You can also encapsulate the detection part of the factory in another helper class that can be used both by the factory and the subclasses (in the sprit of DRY principle).

Mehrdad Afshari
Thanks for your input. So you might recommend that I use a shared helper/utility class to identify the file type from the InputStream in the getImageReader function and just switch on the returned enum?
Venesectrix
Yes, this is a good approach.
Mehrdad Afshari
A: 

I would have a static CanReadFrom method (or something) in the common ImageReader interface (not sure if this is possible -- FIXME). Use reflection to grab all implementors and call the function. If one returns true, return an instance of the class.

strager
+1  A: 

For extensibility, you can externalize some of these dependencies you mention. Like figuring out what kind of file it is or mapping the file type to a class that handles it. An external registry (i.e., properties file) would store say, GIF -> GifReader, or better GIF -> GifMetadataClass. Then your code could be generic and not have dependencies on all the classes, plus you could extend it in the future, or 3rd parties could extend it.

John Ellinwood
I think you are saying that I could use a map to associate a file type with a class that handles that file type, right? How would the factory determine which file type it was based on the InputStream in this case? I'm not sure you could set up a map from the InputStream to the file type.
Venesectrix
From the input stream, a lot of these image classes have magic numbers at the beginning, or at least something generic you could pass through to extract the image type.
John Ellinwood
That's true. I see what you're saying about making it generic and extensible using associations, which is definitely something to consider. In my specific case it may not be worth it, because I only have a few different objects being created and don't anticipate a lot more being added.
Venesectrix
+1  A: 

If this is for windows I would try to guess content type and then use factory. In fact I did this some time ago.

Here is a class to guess content type of a file:

using System;
using System.IO;
using System.Runtime.InteropServices;

namespace Nexum.Abor.Common
{
    /// <summary>
    /// This will work only on windows
    /// </summary>
    public class MimeTypeFinder
    {
        [DllImport(@"urlmon.dll", CharSet = CharSet.Auto)]
        private extern static UInt32 FindMimeFromData(
            UInt32 pBC,
            [MarshalAs(UnmanagedType.LPStr)] String pwzUrl,
            [MarshalAs(UnmanagedType.LPArray)] byte[] pBuffer,
            UInt32 cbSize,
            [MarshalAs(UnmanagedType.LPStr)]String pwzMimeProposed,
            UInt32 dwMimeFlags,
            out UInt32 ppwzMimeOut,
            UInt32 dwReserverd
        );

        public string getMimeFromFile(string filename)
        {
            if (!File.Exists(filename))
                throw new FileNotFoundException(filename + " not found");

            var buffer = new byte[256];
            using (var fs = new FileStream(filename, FileMode.Open))
            {
                if (fs.Length >= 256)
                    fs.Read(buffer, 0, 256);
                else
                    fs.Read(buffer, 0, (int)fs.Length);
            }
            try
            {
                UInt32 mimetype;
                FindMimeFromData(0, null, buffer, 256, null, 0, out mimetype, 0);
                var mimeTypePtr = new IntPtr(mimetype);
                var mime = Marshal.PtrToStringUni(mimeTypePtr);
                Marshal.FreeCoTaskMem(mimeTypePtr);
                return mime;
            }
            catch (Exception)
            {
                return "unknown/unknown";
            }
        }
    }
}
Sergej Andrejev
Do they have any MIME lookup functions in .Net or is Windows API call the only solution short of storing your own lookup table.
James
To my knowledge there is no mime type detection in .NET
Sergej Andrejev
+2  A: 

Both are valid choices depending on context.

IF you're architecting for extensibility - say a plugin model for different ImageReaders - then your Factory class cannot know about all possible ImageReaders. In that case, you go the ImageReader.CanRead(ImageStream) route - asking each implementer until you find one that can read it.

Beware, that sometimes ordering does matter here. You may have a GenericImageReader that can handle JPGs, but a Jpeg2000ImageReader that is better at it. Walking the ImageReader implementers will stop at whichever is first. You may want to look at sorting the list of possible ImageReaders if that's a problem.

Otherwise, if the list of ImageReaders is finite and under your control, then you can go the more traditional Factory approach. In that case, the Factory decides what to create. It's already coupled to the concrete implementations of ImageReader by the ctor, so adding rules for each ImageReader doesn't increase coupling. If the logic to pick a ImageReader is mainly in the ImageReader itself, then to avoid duplication of code, you can still go the ImageReader.CanRead(ImageStream) route - but it could just be hardcoded which types you walk.

Mark Brackett
Thanks for the response! I'm actually using C++, and I believe I only have the option of walking the ImageReader types manually. I don't know of a way to determine what subclasses there are for a base class except to keep track of them specifically in the code (hardcoded or added to an array, etc)
Venesectrix