How can I refactor this to work without breaking the pattern horribly?

Question

I've got a base class object that is used for filtering. It's a template method object that looks something like this.

public class Filter
{
    public void Process(User u, GeoRegion r, int countNeeded)
    {
        List<account> selected = this.Select(u, r, countNeeded); // 1
        List<account> filtered = this.Filter(selected, u, r, countNeeded); // 2

        if (filtered.Count > 0) { /* do businessy stuff */ } // 3

        if (filtered.Count < countNeeded)
            this.SendToSuccessor(u, r, countNeeded - filtered) // 4
    }
}

Select(...), Filter(...) are protected abstract methods and implemented by the derived classes.

1. Select(...) finds objects in the based on x criteria,
2. Filter(...) filters those selected further.
3. If the remaining filtered collection has more than 1 object in it, we do some business stuff with it (unimportant to the problem here).
4. SendToSuccessor(...) is called if there weren't enough objects found after filtering (it's a composite where the next class in succession will also be derived from Filter but have different filtering criteria)

All has been ok, but now I'm building another set of filters, which I was going to subclass from this. The filters I'm building however would require different params and I don't want to just implement those methods and not use the params or just add to the param list the ones I need and have them not used in the existing filters.

They still perform the same logical process though.

I also don't want to complicated the consumer code for this (which looks like this)

Filter f = new Filter1();
Filter f2 = new Filter2();
Filter f3 = new Filter3();

f.Sucessor = f2;
f2.Sucessor = f3;
/* and so on adding filters as successors to previous ones */

foreach (User u in users)
{
    foreach (GeoRegion r in regions)
    {
        f.Process(u, r, ##);
    }
}

How should I go about it?

Answer 1

You could implement the filters as Decorators. In fact, the way you invoke the successor in your filter is exactly like Decorator, so why not implement the pattern fully. The main difference would be that the successor is passed in the constructor. Then you would be able to chain them together like this:

Filter f3 = new Filter3();
Filter f2 = new Filter2(f3);
Filter f = new Filter1(f2);

foreach (User u in users)
{
    foreach (GeoRegion r in regions)
    {
        f.Process(u, r, ##);
    }
}

As you probably are well aware of, you can embed your Template Method into a Strategy: one common base interface, an abstract generic Template Method subclass for each different set of filters, then the set of concrete implementation subclasses for each.

The tough part is the different parameter set. If you want to use the new filters transparently intermixed with the old ones, they should have the same interface, thus they should all get (and pass forward) the superset of all existing parameters, which is rapidly getting ugly over a count of 4-5 parameters.

A possibility would be to pass the special parameters in the constructor of each filter, and the common parameters to process, but this is difficult as you have many runs in a loop with different parameters... Unless you can move the whole nested foreach loop inside the filters, which would then get users and regions as parameters:

public class Filter
{
    private UserCollection users;
    private GeoRegionCollection regions;

    public void Process(UserCollection users, GeoRegionCollection regions)
    {
        this.users = users;;
        this.regions = regions;
    }

    public void Process()
    {
        foreach (User u in users)
        {
            foreach (GeoRegion r in regions)
            {
                Process(u, r, ##);
            }
        }
    }

    public void Process(User u, GeoRegion r, int countNeeded)
    {
        // as before
    }
}

Another possibility might be to group together those parameters in some generic structure which has enough variability to hold your different parameter sets. Typically though, that's going to end up being some ugly map-like storage where you need to look up parameters by name and then downcast them to the right type :-(

Difficult to say more without knowing more detail...

Answer 2

Can you pass the pieces that go to filter as a params collection? Keep everything else the same, but have a collection of possible filter parameters which each subclass knows how to deal with.

Answer 3

Why don't you want to define interface IFilter with one method void Process(User u, GeoRegion r, int countNeeded) and then abstract class FilterType1 (with abstract methods Select and Filter) with two derived classes and another class Filter3 (just implementing Process method)?

Answer 4

It sounds like what you want to do is have the call be this.Filter(selected, u, r, countNeeded, more, parameters, here);. If so, you could implement the additional parameters as parameters to the constructor that are stored as private member fields, like this:

class SuperFilter : Filter
{
    private object more, parameters, here;
    public SuperFilter(object more, object parameters, object here)
    {
        this.more = more;
        this.parameters = parameters;
        this.here = here;
    }
    override protected List<account> Filter(selected, u, r, countNeeded)
    {
        // use more parameters here along with the regular parameters
    }
    ...
}

Answer 5

Since filter takes in different parameter and it always have to execute on a set of data and return the desired result, why dont you use Command Pattern.

Have different command type that each takes in a different set of parameters and always have the method Execute or Process that you call it and always returning a set of data. I suppose you are always dealing with a fixed set of data otherwise you can even use generic and constraint the return type like this:

  public class GregorianFilterCommand<T>: FilterCommand where T is IBusinessEntity
  {
     public GregorianFilterCommand(IList<T> rawDataList, .....);

     public IList<T> Execute();
     .........
  }

Answer 6

I would consider a small refactoring to make use of (if possible) the Intercepting Filter Pattern.

If that doesn't work, because you can't come up with a common interface for all the filters, you can use a Decorator pattern around a Front Controller (as described here) or even apply the Chain of Responsibility pattern, which is very abstract.