HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Factory design for math problems

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
factorydesignmathforproblems

Problem

I'm creating a factory design for math problems. The idea which I have 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, will generate two times.

  • Some problems must be generated more difficult. I pass in the constructor the level and the factory must generate them.



  • To do this, I've got an abstract method called ConfigureLevels, where you has to set up.



  • 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 another's until gets a good problem. A good problem is according to a CRITERIA. I mean, the problem mustn't exist in the list, if we refer that the factory generates FRACTIONS, must be below than 1, etc. And I haven't done yet this last feature.



This is the factory which I'm talking to:

```
public abstract class Factory where T : IProblem
{
protected static Random rnd;
protected LinkedListNode actualNode;
protected LinkedList problems;

public virtual int TotalProblemsPossible { get; set; }
protected virtual int InitialSize { get; set; }

protected Factory(Levels level)
{
this.InitialSize = 20;

Initialize();
ConfigureLevels(level);
FirstTime();
}

public virtual void FirstTime()
{
if (rnd == null) rnd = new Random(100);
if (problems == null)
{
problems = new LinkedList();
Generate(problems);
}

actualNode = problems.First;
}

public virtual T CreateProblem()
{
if (actualNode.Next == null)
{
Generate(problems);
}

actualNode = actualNode.Next;
return actualNode.Value;
}

private void Generate(LinkedList problems)
{
f

Solution

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();

Code Snippets

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();

Context

StackExchange Code Review Q#8041, answer score: 3

Revisions (0)

No revisions yet.