开发者

How to design domain model with interfaces that are dependent on the model's properties

I have a domain model (see example below) that has several interfaces that are determined based upon the value of properties within the model itself. While the code below "works" it doesn't feel right. I am sure that there is probably a better approach, but I haven't been able to come up with one. I would be very interested in getting feedback with some alternate approaches.

public interface IPickListGenerator
    {
        void Execute();
    }

public class SportsPicklistGenerator : IPickListGenerator
{
        public void Execute()
        {
            // Do something
        }
}

public class EntertainmentPicklistGenerator : IPickListGenerator
{
    public void Execute()
    {
        // Do something
    }
}

public interface IQuestionIsAnswerableDeterminer
{
    void Execute();
}

public class GameQuestionIsAnswerableDeterminer : IQuestionIsAnswerableDeterminer
{
    public void Execute()
    {
        // Do something
    }
}

public class RoundQuestionIsAnswerableDeterminer : IQuestionIsAnswerableDeterminer
{
    public void Execute()
    {
        // Do something
    }
}

public class Pool
{
    public enum PoolType
    {
        Sports,
        Entertainment
    }

    public enum DeadlineType
    {
        Game,
        Round
    }

    private IPickListGenerator mPickListGenerator = null;
    private IQuestionIsAnswerableDeterminer mQuestionIsAnswerableDeterminer = null;

    public PoolType Type { get; set; }
    public DeadlineType Deadline { get; set; }

    public IQuestionIsAnswerableDeterminer IsQuestionAnswerableDeterminer
    {
        get
        {
            if (mQuestionIsAnswerableDeterminer == null) SetPoolSpecificInterfaces();
            return mQuestionIsAnswerableDeterminer;
        }
    }

    public IPickListGenerator PickListGenerator
    {
        get
        {
            if (mPickListGenerator == null) 开发者_JAVA技巧SetPoolSpecificInterfaces();
            return mPickListGenerator;
        }
    }

    private void SetPoolSpecificInterfaces()
    {
        switch (Type)
        {
            case Pool.PoolType.Sports:
                mPickListGenerator = new SportsPicklistGenerator();
                break;
            case Pool.PoolType.Entertainment:
                mPickListGenerator = new EntertainmentPicklistGenerator();
                break;
        }

        switch (Deadline)
        {
            case Pool.DeadlineType.Game:
                mQuestionIsAnswerableDeterminer = new GameQuestionIsAnswerableDeterminer();
                break;
            case Pool.DeadlineType.Round:
                mQuestionIsAnswerableDeterminer = new RoundQuestionIsAnswerableDeterminer();
                break;
        }
    }
}
// Example usages:
    var pool = new Pool{ Type = Pool.PoolType.Sports, Deadline = Pool.DeadLineType.Game};
    pool.IsQuestionAnswerableDeterminer.Execute();
    pool.PickListGenerator.Execute();


Other than potential logical inconsistencies (subsequently setting Type or Deadline will result in the "incorrect" value for your determiner and generator being supplied back to the user), I don't see anything wrong with the design from a larger perspective. This is very similar to the way a lot of factory patterns work: an external interface (or base class) that's implemented by multiple internal concrete classes, the correct instance of which is determined by some sort of discriminating value.

Unless the construction is expensive (and won't always be necessary), I'd suggest moving to smarter properties for Type and Deadline that set the appropriate interface implementation variables in their set blocks rather than loading them lazily in the other properties as you do now.

If the construction is expensive, I would still switch to smarter properties for Type and Deadline and clear out (and do any necessary clean up on) the values that might have been previously set.


You probably need to remove the dependencies from your code. Excessiveness from your code exists is in 2 ways:

  • you have an excessive code (interfaces)
  • you have an excessive constraints (enums)

Finally, I've got this, in dialog with Adam, which eliminates exception throws, the need of two switch statement, two enumerations, whilst preserving pool closures and elimination possible errors in code. For each switch statement (each interface implementation pair), you have to implement its own static method instantiating Pool class properly:

public interface IPool
{
    void Execute();
}

public class Pool : IPool
{
    private Pool() { }

    public IPickListGenerator PickListGenerator { set; private get; }
    public IQuestionIsAnswerableDeterminer IsQuestionIsAnswerableDeterminer { set; private get; }

    public static IPool GetSportsGame() 
    {
        return new Pool
        {
            PickListGenerator = new SportsPicklistGenerator(),
            IsQuestionIsAnswerableDeterminer = new GameQuestionIsAnswerableDeterminer()
        };
    }
    public static IPool GetSportsEntertainment() 
    {
        return new Pool
        {
            PickListGenerator = new SportsPicklistGenerator(),
            IsQuestionIsAnswerableDeterminer = new EntertainmentPicklistGenerator()
        };
    }
    public void Execute()
    {
        IsQuestionIsAnswerableDeterminer.Execute();
        PickListGenerator.Execute();
    }
}

--- History of that post, leading to this solution:

You can improve your code by simplifying the code, because in your implementation, Pool class is limited to enumerations, and if so, the Pool class is not extensible. You probably will have to rewrite SetPoolSpecificInterfaces() again and again, when new requirements and interface implementation comes into view. So the weakest point of the current design is that particular SetPoolSpecificInterfaces() method and if so, not extensible Pool class.

If that is what you required, than interfaces is not really needed. In other case, you will not be dependent upon number of interface implementations (which is actually fixed by the corresponding enum) so, it probably can be be simplified to following code (i fixed the issues), removing these constraints:

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            // Example usages:
            var pool = new Pool
            {
                PickListGenerator = new SportsPicklistGenerator(),
                IsQuestionIsAnswerableDeterminer = new GameQuestionIsAnswerableDeterminer()
            };
            pool.ExecuteIsQuestionIsAnswerableDeterminer();
            pool.ExecutePickListGenerator();
        }
    }

    public class Pool
    {
        public IPickListGenerator PickListGenerator { set; private get; }
        public IQuestionIsAnswerableDeterminer
            IsQuestionIsAnswerableDeterminer { set; private get; }

        public void ExecuteIsQuestionIsAnswerableDeterminer()
        {
            IsQuestionIsAnswerableDeterminer.Execute();
        }

        public void ExecutePickListGenerator()
        {
            PickListGenerator.Execute();
        }
    }
}

You probably can stop here, but what if you will always call ExecuteIsQuestionIsAnswerableDeterminer() before ExecutePickListGenerator() call? then you can refacor code as follows:

    public class Pool
    {
        public IPickListGenerator PickListGenerator { set; private get; }
        public IQuestionIsAnswerableDeterminer 
            IsQuestionIsAnswerableDeterminer { set; private get; }

        public void Execute()
        {
            // improvements, seen by Arnis
            if (IsQuestionIsAnswerableDeterminer == null)
            {
                thrown new ArgumentException("IsQuestionIsAnswerableDeterminer");
            }
            if (PickListGenerator == null)
            {
                thrown new ArgumentException("PickListGenerator");
            }
            IsQuestionIsAnswerableDeterminer.Execute();
            PickListGenerator.Execute();
        }
    }

If so, you can write something looks like this:

var pool = new Pool
{
    PickListGenerator = new SportsPicklistGenerator(),
    IsQuestionIsAnswerableDeterminer = new GameQuestionIsAnswerableDeterminer()
};
pool.Execute();

// reuse the same class logic in Pool class in Execute method
pool.PickListGenerator = new GamePickListGenerator();
pool.Execute();

// reuse the same class logic in Pool class in Execute method
pool.IsQuestionIsAnswerableDeterminer = new RoundQuestionIsAnswerableDeterminer();
pool.Execute();

As @Adam Robinson said, it changes some logic of Pool (to fix this you can use close Pool implementation), so you can fix it in this way, introducing new interface, IPool, adding private constructor to Pool class and static logic instantiation, that will eliminate the need of a swith statement completely:

public interface IPool
{
    void Execute();
}

public class Pool : IPool
{
    private Pool() { }

    public IPickListGenerator PickListGenerator { set; private get; }
    public IQuestionIsAnswerableDeterminer IsQuestionIsAnswerableDeterminer { set; private get; }

    public static IPool GetSportsGame() 
    {
        return new Pool
        {
            PickListGenerator = new SportsPicklistGenerator(),
            IsQuestionIsAnswerableDeterminer = new GameQuestionIsAnswerableDeterminer()
        };
    }
    public static IPool GetSportsEntertainment() 
    {
        return new Pool
        {
            PickListGenerator = new SportsPicklistGenerator(),
            IsQuestionIsAnswerableDeterminer = new EntertainmentPicklistGenerator()
        };
    }
    public void Execute()
    {
        IsQuestionIsAnswerableDeterminer.Execute();
        PickListGenerator.Execute();
    }
}

This was edited by Arnis:

public class Pool{
  private readonly IPickListGenerator _generator;
  private readonly IQuestionIsAnswerableDeterminer _determiner;
  public Pool(IPickListGenerator generator, 
    IQuestionIsAnswerableDeterminer determiner){

    if(determiner==null||generator==null)
      throw new ArgumentNullException();
    _generator=generator;
    _determiner=determiner;
  }

  public void Execute(){
    _determiner.Execute();
    _generator.Execute();
  }
}

var generator=new SportsPicklistGenerator();
var determiner=new GameQuestionIsAnswerableDeterminer();
var pool = new Pool(generator, determiner);
pool.Execute();
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜