Does this follow abstract factory pattern

2019-08-06 07:38发布

问题:

As per defination "Design Patterns: Abstract Factory". informIT. Archived from the original on 2009-10-23. Retrieved 2012-05-16,Object Creational -> Abstract Factory:.

Intent: Provide an interface for creating families of related or dependent objects without specifying their concrete classes."

Below is my try at abstract design pattern. This is my first hands on factory method. Can anyone please help me on this.

enum ProductType
{
    BeautySoap = 1,
    DetergentSoap = 2,
    HairWax = 3,
    BodyWax = 4,
}
interface ISoap
{
    string Create(string name);

}
interface IWax
{
    string Create(string name);
}

public class HarWax : IWax
{
    public string Create(string name)
    {
        return string.Format("Hair Wax {0} created", name);
    }
}

public class BodyWax : IWax
{

    public string Create(string name)
    {
        return string.Format("Body wax {0} created", name);
    }
}
public class BeautySoapFactory : ISoap
{
    public string Create(string name)
    {
         return string.Format("Toilet soap {0} created", name);
    }

}
public class DetergentSoapFactory : ISoap
{
    public string Create(string name)
    {
         return string.Format("Detergent bar {0} created", name);
    }
}



//factory of factories(Bike Factory, Scooter Factory)
/// <summary>
/// The 'AbstractFactory' interface. 
/// </summary>
interface IBeautyProduct
{
    ISoap CreateSoap(ProductType type);
    IWax CreateWax(ProductType type);
}

class HULFactory : IBeautyProduct
{
    public ISoap CreateSoap(ProductType type)
    {
        switch (type)
        { 
            case ProductType.BeautySoap:
                return new BeautySoapFactory();
            case ProductType.DetergentSoap:
                return new DetergentSoapFactory();
            default:
                throw new ApplicationException(string.Format("Soap '{0}' cannot be created", type));
                //Console.WriteLine(string.Format("Soap '{0}' cannot be created", name));
                //break;
        }
    }
    public IWax CreateWax(ProductType type)
    {
        switch (type)
        {
            case ProductType.HairWax:
                return new HarWax();
            case ProductType.BodyWax:
                return new BodyWax();
            default:
                throw new ApplicationException(string.Format("Wax '{0}' cannot be created", type));
        }
    }
}

class LotusherbalsFactory : IBeautyProduct
{
    public ISoap CreateSoap(ProductType type)
    {
        switch (type)
        {
            case ProductType.BeautySoap:
                return new BeautySoapFactory();
            case ProductType.DetergentSoap:
                return new DetergentSoapFactory();
            default:
                throw new ApplicationException(string.Format("Soap '{0}' cannot be created", type));
            //Console.WriteLine(string.Format("Soap '{0}' cannot be created", name));
            //break;
        }
    }
    public IWax CreateWax(ProductType type)
    {
        switch (type)
        {
            case ProductType.HairWax:
                return new HarWax();
            case ProductType.BodyWax:
                return new BodyWax();
            default:
                throw new ApplicationException(string.Format("Wax '{0}' cannot be created", type));
        }
    }
}

回答1:

It's a fictional example (at least it seems to me) so it's hard to give really useful suggestions but something comes to my eyes immediately: that long verbose error-prone switch statements.

First of all don't think that to follow this pattern you must use interfaces. Also an abstract base class is fine, what is better to use is a vast topic already discussed in thousands forums, posts and tutorials.

To understand why switch is bad let me simplify your scenario (you have factories of factories...), let's assume you just have one:

class Factory 
{
    public ISoap CreateSoap(ProductType type)
    {
        switch (type)
        {
            case ProductType.BeautySoap:
                return new BeautySoapFactory();
            case ProductType.DetergentSoap:
                return new DetergentSoapFactory();
            default:
                throw new ApplicationException();
        }
    }
}

What's wrong in this code?

You're throwing wrong exception type, for an unknown or invalid enum value you should use InvalidEnumArgumentException:

throw new InvalidEnumArgumentException("Unknown product type.", type, typeof(ProductType));

Second mistake is to put together different things in same enum. You have soap and wax. They're completely unrelated and you even have two different factory methods to create them. This is confusing (in a more complex scenario when you have a ProductType parameter you may expect to be free to use them all) then let's split them:

public enum SoapTypes {
    Beauty,
    Detergent
}

Your factory method will then be:

public ISoap Create(SoapType type)

Note that you don't even need different names for your methods because you now have different parameter types. You default case will now be executed only for unknown values (you add a new item to SoapType but you forget to update your code) or for clearly wrong usage (for example calling Create((SoapType)100) where 100 isn't a literal (!!!) value but it's calculated/read somewhere).

Now let's imagine you add a new product type, you have to do all of these:

  • Create a new class to implement ISoap.
  • Add a new value to ProductType.
  • Add a new case to your switch.

You have one step more than what should be necessary. Ideally each change should affect only one place in your code, here you can't avoid first two steps but third one may be superfluous. You have few options.

If this code is performance critical (measure!) you may use a dictionary to have more readable code (IMO):

static Dictionary<SoapType, Type> _soaps = new Dictionary<SoapType, Type>
{
    { SoapType.Beauty, typeof(BeautySoapFactory) }
    { SoapType.Detergent, typeof(DetergentSoapFactory) }
};

public ISoap CreateSoap(SoapType type) {
    return (ISoap)Activator.CreateInstance(_soaps[type]);
}

It looks more readable (to me) and it's open to dynamic changes (class to instantiate may be not hard-coded in your source files but come from configuration). You would probably use this method if mapping is configurable.

However you still have to update your enum and your factory method/dictionary. Probably they're in different source files and you may forget something. If performance isn't a (measured!) issue you may want to add metadata to your enum values, it'll be hard to forget it when you have everything on the same screen:

public enum SoapType {
    [FactoryClass(typeof(BeautySoapFactory)] Beauty,
    [FactoryClass(typeof(DetergentSoapFactory)] Detergent,
}

Your factory method will be:

public ISoap CreateSoap(SoapType type) {
    var descriptor = FactoryClassAttribute.From(type);
    return (ISoap)Activator.CreateInstance(descriptor.Type);
}

FactoryClassAttribute is, for example, this (note that this code may be greatly generalized). Note that enum constants are fields:

[Serializable]
sealed class FactoryClassAttribute : Attribute {
    public FactoryClassAttribute(Type type) {
        Type = type;
    }

    public Type Type { get; set; }

    public static FactoryClassAttribute From(Enum value) {
    {
        var type = value.GetType();
        return type.GetField(Enum.GetName(type, value))
            .GetCustomAttributes(false)
            .OfType<FactoryClassAttribute>()
            .FirstOrDefault();
    }    
}


回答2:

You mention Abstract Factory, Factory Method and then you have this code, which is a Simple Factory:

public ISoap CreateSoap(ProductType type)
{
    switch (type)
    { 
        case ProductType.BeautySoap:
            return new BeautySoapFactory();
        case ProductType.DetergentSoap:
            return new DetergentSoapFactory();
        default:
            throw new ApplicationException(string.Format("Soap '{0}' cannot be created", type));
            //Console.WriteLine(string.Format("Soap '{0}' cannot be created", name));
            //break;
    }
}

Can anyone please help me on this.

These three design patterns are easily confused. It's hard to answer your question (there's not really a clear question).

Rather than repeat what all three patterns are in an answer (that would be very long), I'll refer you to a pretty good explanation of all three variants:

http://corey.quickshiftconsulting.com/blog/first-post

A few hints to help distinguish them:

  • Simple factory is very common, and usually involves a method (not polymorphic!) to return a subtype based on some discriminant. Your CreateSoap() is a perfect example.
  • Factory method pattern is less common than Simple factory, and involves the use of a polymorphic method where each subtype returns an instance of a different created type. A great example is Java's Collection.iterator() method. The subtype (implementation) of Collection returns an implementation of Iterator that corresponds to the Collection subtype. For example, an ArrayList.iterator() will return a (hidden) type that knows how to iterate an ArrayList (which is different than a LinkedList).
  • Abstract factory is probably the least common, and it involves a family of product types, and usually relies on Factory methods within to create subtypes.

If you like pizza, Head First Design Patterns has all its code examples on GitHub. Here are the links to the specific examples related to your question:

  • Simple Factory
  • Factory Method - PizzaStore.createPizza(...) is the factory method defined in the various subclasses e.g., ChicagoPizzaStore, NYPizzaStore
  • Abstract Factory - PizzaIngredientFactory defines the interface of how to make a family of related products, e.g., createDough(), createSauce(), etc.