Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Design pattern question for maintainability

I'm not sure if there is a pattern that should be used here but here is the situation:

I have a number of concrete classes that implement an interface:

public interface IPerformAction
{
   bool ShouldPerformAction();
   void PerformAction();
}

I have another class that checks input to determine if ShouldPerformAction should be execute. The rub is that new checks are added fairly frequently. The interface for the checking class is defined as follows:

public interface IShouldPerformActionChecker
{
   bool CheckA(string a);
   bool CheckB(string b);
   bool CheckC(int c);
   // etc...
}

Finally I currently have the concrete classes call each of the checker methods with the data specific to that concrete class:

public class ConcreteClass : IPerformAction
{
   public IShouldPerformActionCheck ShouldPerformActionChecker { get; set; }

   public string Property1 { get; set; }
   public string Property2 { get; set; }
   public int Property3 { get; set; }

   public bool ShouldPerformAction()
   {
      return 
         ShouldPerformActionChecker.CheckA(this.Property1) ||
         ShouldPerformActionChecker.CheckB(this.Property2) ||
         ShouldPerformActionChecker.CheckC(this.Property3);
   }

   public void PerformAction()
   {
      // do something class specific
   }
}

Now each time I add a new check, I have to refactor the concrete classes to include the new check. Each concrete class passes different properties to the checking method so subclasses the concrete classes is not an option. Any ideas on how this could be implemented in a cleaner manner?

like image 254
bmancini Avatar asked Dec 16 '09 01:12

bmancini


2 Answers

Let's take a step back - why are you using interfaces in the first place? Can a single implementation of IShouldPerformActionCheck be shared between multiple implementations of IPerformAction? It seems the answer is no, since ICheck must be aware of implementation-specific properties (Property1, Property2, Property3) on the Action in order to perform the check. Therefore the relationship between IAction and ICheck requires more information than the IAction contract can provide to ICheck. It seems your Check classes should be based on concrete implementations that are coupled to the specific type of action they check, like:

abstract class CheckConcreteClass
{
    abstract void Check(ConcreteClass concreteInstance);
}
like image 76
Rex M Avatar answered Sep 21 '22 21:09

Rex M


The "CheckA", "CheckB", etc. names, presumably chosen to avoid exposing confidential information, also obfuscate the nature of the relationships between classes, so I'll have to wing it.

However, this is very nearly double dispatch, except you are performing conversion of the objects in-between.

EDIT: Try playing the double dispatch pattern "by the book", rather than decomposing the objects mid-dispatch. To do this, you'd need something like the following:

public interface IPerformAction
{
    bool ShouldPerformAction(IShouldPerformActionChecker checker);
    void PerformAction();
}

public interface IShouldPerformActionChecker
{
    bool CheckShouldPerformAction(FloorWax toCheck);
    bool CheckShouldPerformAction(DessertTopping toCheck);
    // etc...
}

public class FloorWax : IPerformAction
{
    public string Fragrance { get; set; }

    // Note that the text of this method is identical in each concrete class,
    // but compiles to call a different overload of CheckShouldPerformAction.
    public bool ShouldPerformAction(IShouldPerformActionChecker checker)
    {
        return checker.CheckShouldPerformAction(this);
    }
}

public class DessertTopping: IPerformAction
{
    public string Flavor { get; set; }

    // Note that the text of this method is identical in each concrete class,
    // but compiles to call a different overload of CheckShouldPerformAction.
    public bool ShouldPerformAction(IShouldPerformActionChecker checker)
    {
        return checker.CheckShouldPerformAction(this);
    }
}

public class ShimmerApplicationChecker : IShouldPerformActionChecker
{
    // handles binding of FloorWax class to required checks
    public bool CheckShouldPerformAction(FloorWax toCheck)
    {
        return CheckAroma(toCheck.Fragrance);
    }

    // handles binding of DessertTopping class to required checks
    public bool CheckShouldPerformAction(DessertTopping toCheck);
    {
        return CheckAroma(toCheck.Flavor);
    }

    // some concrete check
    private bool CheckAroma(string aroma)
    {
        return aroma.Contains("chocolate");
    }
}
like image 41
Jeffrey Hantin Avatar answered Sep 18 '22 21:09

Jeffrey Hantin