2
\$\begingroup\$

The problem statement that I am trying to solve is this;

We will retrieve a list of Tasks from an external repository. These tasks will have a type and for each type, there needs to be a specific implementation that defines the Model it needs it order to perform the Task and then the logic needed to Process the task, along with any dependencies.

At one point, the Factory was setup to use Generics, but reverted to this as it seemed simpler.

 public interface ITask 
 {
     bool ProcessTask();
 }

 public interface ITask <TModel> 
 {

 }

 public interface IModelBuilder <out TModel> 
 {
     TModel Build(ITaskDefinition taskDefinition);
 }

 public abstract class BaseTask <TModel> : ITask<TModel> , ITask where TModel: IModel 
 {
     public TModel Model {  get; set;}

     public ITaskDefinition TaskDefinition { get; set; }

     protected BaseTask(ITaskDefinition definition, IModelBuilder <TModel> builder) 
     {
         Model = builder.Build(definition);
     }

     public abstract bool ProcessTask();
 }


 public interface ITaskFactory 
 {
     ITask GetTask(ITaskDefinition taskDefinition, TaskType type);
 }

Concrete classes

public class TaskFactory: ITaskFactory
{
    private readonly IContainer _container;

    public TaskFactory(IContainer container)
    {
        _container = container;
    }


    public ITask GetTask(ITaskDefinition taskDefinition, TaskType type)
    {
        return _container
            .With(taskDefinition)
            .GetInstance < ITask > (type.ToString());
    }
}

public class DateTask: BaseTask < DateModel >
{
    private readonly ITaskManagementFacade _taskManagementFacade;

    public CaptureCompletionDateTask(ITaskDefinition taskDefinition, IModelBuilder < DateModel > builder, ITaskManagementFacade taskManagementFacade): base(taskDefinition, builder)
    {
        _taskManagementFacade = taskManagementFacade;
    }
    public override bool ProcessTask()
    {
        return _taskManagementFacade.CompleteTask(Model.SomeDate < DateTime.Now);
    }

}

public class DateModelBuilder: IModelBuilder < DateModel >
{
    private readonly ITaskManagementFacade _taskManagementFacade;


    public DateModelBuilder(ITaskManagementFacade taskManagementFacade)
    {
        _taskManagementFacade = taskManagementFacade;
    }

    public DateModel Build(ITaskDefinition taskDefinition)
    {
        var model = new DateModel
        {
            MyDate = DateTime.Now,
                Notes = _taskManagementFacade.GetTask((Guid.NewGuid()))
        };

        return model;
    }
}


public class DateModel: IModel
{
    public DateTime MyDate
    {
        get;
        set;
    }

    [Required]
    public string Notes
    {
        get;
        set;
    }
}

StructureMap container config

config.For<ITask>().Use<CaptureCompletionDateTask>().Named(TaskType.Task1.ToString());
config.For<IModelBuilder<DateModel>>().Use<DateModelBuilder>();

Code to get Task

var task = _taskFactory.GetTask(taskDefinition, TaskType.Task1);

What am I looking for? General comments, this is my first attempt at this kind of object construction. It would be great if I could get around the Named config in Structuremap and go back to using a Generic Factory to Load a Task based on TaskType.

Thanks

\$\endgroup\$
1
\$\begingroup\$

I believe it is not good to pass builder to constructor as an argument. I recommend to read Constructor Design guidelines from Microsoft. If you have any non-simple calculation in constructor it will be better to move it to a static method. In your case I would firstly create model and then pass the built one to the constructor. Unfortunately I don't see where and how a builder is created so maybe your solution keeps a good balance between complexity, flexibility and beauty.

It seems that here

public TModel Model {  get; set;}

you don't need setter since Model is set via constructor and is not supposed to be changed.

Small note: don't use extra spaces in generics :) IModelBuilder < DateModel > will look better as IModelBuilder<DateModel>.

\$\endgroup\$
  • \$\begingroup\$ The builder in the constructor argument is for the DI container to inject the instance of it automatically. Fair point on the model setter. The code was formatted online as when I pasted it directly from solution, it looked worse, there are no spaces in the solution :) \$\endgroup\$ – ChrisBint Aug 4 '17 at 6:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.