How to improve my factory design?

Question

I'm creating a factory design for math problems. The purpose of the factory is:

• When the factory initializes, creates in a list some problems (20 initially)
• If the program wants more than 20, the list should grow until reach the requested quantity.

For example if I require 30 problems of the X problem, it will generate two times.

• The problems must be generated with a certain difficulty (or level). To do this, I've got an abstract method called ConfigureLevels.
• I set an abstract method called Generate, this one must be implemented in a concrete class.
• When the problem is generated, sometimes is not a good problem. When this happen, it must generate anothers until gets a good problem. A good problem is according to a criteria.

This is the factory design which I'm talking to.

Here is the factory list. Count property is for How many problems would you like to generate? and Problems returns a list of problems until accomplishes the quantity requested.

public class ProblemListFactory
{
    LinkedList<Problem> problems;
    LinkedListNode<Problem> current;
    ProblemFactory problemFactory;

    public IEnumerable<Problem> Problems
    {
        get
        {
            while (problems.Count < Count)
                Generate();

            return problems.Take(Count);
        }
    }

    public int Count { get; set; }

    public ProblemListFactory(ProblemFactory problemFactory)
    {
        if (problemFactory == null) throw new ArgumentNullException("problemFactory");

        problems = new LinkedList<Problem>();
    }

    public void Generate()
    {
        int i = 0;
        while (i < problemFactory.GrowSize)
        {
            Problem problem = problemFactory.Generate();
            if (problems.Contains(problem) || !problemFactory.IsValid(problem))
                continue;

            problems.AddLast(problem);
            i++;
        }
    }
}

Here is the abstract ProblemFactory.

• The abstract factory provide a Random variable.
• GrowSize is a default size. When the factory list needs to generate more problem, will use this size.
• MaxCapacity property is the limit of problems that can be generated.
• Also contains a CanConfigureXLevel() which by default returns false, but if you want that it'll be available, just override it to true. ConfigureXLevel() is an abstract method which knows how to configure the level.

_

public abstract class ProblemFactory
{
    protected IDictionary<Levels, IConfiguration> Configurations = new Dictionary<Levels, IConfiguration>();
    protected static Random Random = new Random();


    public virtual int GrowSize
    {
        get { return 20; }
    }

    public abstract int MaxCapacity { get; }

    public ProblemFactory() { }
    public ProblemFactory(Levels level)
    {
        //Configure(level);
    }

    public abstract Problem Generate();

    /* Some classes need to check if the problem accomplishes the requirements.
     * I.E. If we generate a fraction class and we want its decimal value is below than 1,
     * here is the correct place to put the condition */
    public virtual bool IsValid(Problem p)
    {
        return true;
    }

    public virtual bool CanConfigureEasyLevel()
    {
        return false;
    }

    public virtual bool CanConfigureMediumLevel()
    {
        return false;
    }

    public virtual bool CanConfigureHardLevel()
    {
        return false;
    }

    protected abstract void ConfigureEasyLevel();
    protected abstract void ConfigureMediumLevel();
    protected abstract void ConfigureHardLevel();

    private void Configure(Levels level)
    {
        switch (level)
        {
            case Levels.Easy:
                if (!CanConfigureEasyLevel()) throw new InvalidOperationException("Level not available");
                ConfigureEasyLevel();

                break;
            case Levels.Medium:
                if (!CanConfigureEasyLevel()) throw new InvalidOperationException("Level not available");
                ConfigureMediumLevel();

                break;
            case Levels.Hard:
                if (!CanConfigureEasyLevel()) throw new InvalidOperationException("Level not available");
                ConfigureHardLevel();

                break;
            default:
                throw new InvalidOperationException("level");
        }
    }
}

And here is a concrete class, check how I'm overriding some ConfigureXLevel from the abstract ProblemFactory. The factory create additions problems, and it should know how to calculates. (it's not completed yet).

 public class AdditionProblemFactory : ProblemFactory
{
    //BinaryProblemConfiguration configuration;

    public AdditionProblemFactory(Levels level)
        : base(level)
    {
        // ... 
    }

    public override Problem Generate() 
    {
        int x = 2;
        int y = 3;
        Operators op = Operators.Addition;

        //return new BinaryProblem(x, y, operator, answer); 
        return ProblemA.CreateProblemA(x, y, op);
    }

    public override int MaxCapacity
    {
        get { return 50; }
    }

    public override bool CanConfigureMediumLevel()
    {
        return true;
    }

    protected override void ConfigureEasyLevel()
    {
        throw new NotImplementedException();
    }

    protected override void ConfigureMediumLevel()
    {
        // Bound<T> = ..
    }

    protected override void ConfigureHardLevel()
    {
        throw new NotImplementedException();
    }
}

Until now, the design has improved a lot. (Thanks Erik)

Now I need to know a list of available levels. I supposed create three three virtual functions CanConfigureXLevel is not a good way.

EDIT: I agree with you Erik, I think it'll be great to create some kind of dictionary which contains the available levels (Level enum as key and value will be a configuration which works like a container of objects usefull to generate the problem (for example binary and times tables both needs two bound objects)).

According to your explanation Erik, create a Dictionary<Levels, IConfiguration> configurations; in ProblemFactory class. At this case, for binary problems we can create BinaryProblemConfiguration which contains Bound1, Bound2 and Operators (operators is a list if we want to have a mix of additions, substractions, multiplications or divisions). I guess doing this, we don't need CanConfigureXLevel and ConfigureXLevel, and the idea would just adding values to the dictionary, and here is I don't know what I should have to modify.

Here is the current project (Factories => Infrastructure.FactoryCore | Configurations => Infrastructure.ConfigurationCore | BinaryProblem => ExerciseA):

http://www.mediafire.com/?fv1n4iym3xxxglo

Answer 1

I think your list factory shouldn't be responsible for creating the concrete problems. How about separating the list factory from the problem factory? That way you can have timeTableProblems and binaryProblems in the same list. (even though that might not be a requirement, it's still a nice separation of concerns)

It actually looks like you've catered for this since you have the IProblem interface. The list factory wouldn't have to be generic either, it'd just keep a list of IProblems generated by whatever problem factories it's given.

Also, I believe you could apply the template method pattern to some of your methods. There is some repeated code in each of your concretes.

For instance you could pull the ConfigureLevels method up to the base class and call down to ConfigureEasy, ConfigureMedium and ConfigureHard.

Update

Regarding template methods, the point is to remove duplication, and at the same time avoid letting the FirstTime method be overridden for instance. It should call out to an empty OnFirstTime method instead. Derivatives might just forget to call base.FirstTime, and you probably don't want that. (Why not just join it with Initialize() ? )

Here's a stub of a sample of separating the list factory and the problem factories. The point is that these are two different things. A list of problems, and the problems themselves.

Also have a look at this: http://en.wikipedia.org/wiki/Single_responsibility_principle

public class ProblemListFactory
{ 
    // omitted stuff

    protected Factory(ProblemFactory problemFactory) 
    { 
         // ...
    } 

    public void Generate()
    {
        for(// ...)
        {
            IProblem problem = problemFactory.Generate();
            if (ProblemExists(problem))
                continue;
            problems.Add(problem);
        }
    }
}

public abstract class ProblemFactory
{
    protected Random Random = new Random();

    public abstract IProblem Generate();
}

public class BinaryProblemFactory : ProblemFactory
{
    public BinaryProblemFactory(Level level)
    {
        // ...
    }

    public override IProblem Generate()
    {
        // ...
        return new BinaryProblem(x, y, operator, answer);
    }
}

// ...
var binaryProblemListFactory = new ProblemListFactory(new BinaryProblemFactory(Level.Easy));
var binaryProblems = binaryProblemListFactory.Generate();
var timeTableProblemListFactory = new ProblemListFactory(new TimeTableProblemFactory(Level.Medium));
var timeTableProblems = timeTableProblemListFactory.Generate();