Advice on "Factory" Pattern Implementation I'm refactoring some code around a couple of ASP.NET Web Forms pages and decided to try out a variation of the Abstract Factory pattern of my own design. I need to create an implementer of an abstract class based on what query string value is available.

Here's (a simplified version of) what I have right now:

internal static class Factory
{
   public static AbstractClass Create(NameValueCollection queryString)
   {
       if (queryString["class1"] != null)
       {
            return new Class1(queryString["class1"]);
       }
       if (queryString["class2"] != null)
       {
           return new Class2(queryString["class2"]);
       }
       if (queryString["class3"] != null)
       {
           return new Class3(queryString["class3"]);
       }

       return null;
   }
}


I don't know how I feel about this implementation. It certainly is better than what was there before, but I have a strong sense this could be improved some how. 

So now, in the relevant pages, I can simply do this:

AbstractClass item = Factory.Create(Request.QueryString);
item.DoStuff();


Does anyone have any suggestions? Or would this be considered good as-is?
I Think you should do it more generic:

public static class Factory
{
    static Dictionary&lt;String, Func&lt;String, Abstract&gt;&gt; classes;

    public static Abstract GetInstance(String s){
        if(!classes.ContainsKey(s)) throw new Exception();
        Func&lt;String, Abstract&gt; create=classes[s];
        return create(s);
    }

    static Factory ()
    {
        classes=new Dictionary&lt;String, Func&lt;String, Abstract&gt;&gt;(){
            {"class1", (x)=&gt;new Class1(x)},
            {"class2", (x)=&gt; new Class2(x)}
        };
    }
}


This code is way more readable.
You have the static constructor block, where you register the factory methods according to the keys. And the GetInstance-Method shrinks to 3 lines, of which one is a purely guarding clause.

Edit: modified after Mat's Mug's hint.

P.S: Oh, I forgot to mention, that you really never should return null. 

Better a) throw an Exception or b) use a NullObject.