4

I'm implementing the template method pattern. There are a couple of settings that clients can change on this template method; but I would like to prevent implementers of the template method from changing these settings, because in such cases they might be able to break invariants of the template method (base) class. I'd like such things to fail to compile if possible. (Otherwise tis time to refactor to strategy :) )

Example:

abstract class TemplateMethod
{
    // Clients of implementers of this template method change these settings before
    // calling GetItems()
    public SomeType RootItem { get; set; }
    public bool Recursive { get; set; }
    protected abstract bool ItemIsWhitelisted(SomeType item);
    public IEnumerable<SomeType> GetItems()
    {
        var stack = new Stack<T>();
        stack.Push(RootItem);
        while (!stack.empty())
        {
            var current = stack.Pop();
            if (Recursive)
            {
                foreach (var item in current.Children)
                    stack.Push(item);
            }

            if (!ItemIsWhitelisted(current))
                yield return current;
        }
    }
}

class Implementer : TemplateMethod
{
    protected override bool ItemIsWhitelisted(SomeType item)
    {
        Recursive = false; // Oops; TemplateMethod.GetItems didn't expect this
                           // to change inside ItemIsWhitelisted
        return item.CanFrobinate();
    }
}

One method is to refactor to strategy, producing the following:

interface IImplementer
{
    bool ItemIswhitelisted(SomeType item);
}

sealed class NoLongerATemplateMethod
{
    // Clients of implementers of this template method change these settings before
    // calling GetItems()
    public SomeType RootItem { get; set; }
    public bool Recursive { get; set; }
    public IImplementer impl { get; set; } // would be private set in real code
    public IEnumerable<SomeType> GetItems()
    {
        var stack = new Stack<T>();
        stack.Push(RootItem);
        while (!stack.empty())
        {
            var current = stack.Pop();
            if (Recursive)
            {
                foreach (var item in current.Children)
                    stack.Push(item);
            }

            if (!impl.ItemIsWhitelisted(current))
                yield return current;
        }
    }
}

class Implementer : IImplementer
{
    public bool ItemIsWhitelisted(SomeType item)
    {
        Recursive = false; // No longer compiles
        return item.CanFrobinate();
    }
}

I'm curious if there's a language feature which indicates this constraint without applying refactor to strategy.

Billy ONeal
  • 104,103
  • 58
  • 317
  • 552
  • 1
    Could you provide a piece of pseudo-code, which is doing what you want? – Dennis Oct 10 '12 at 06:29
  • Are you worried that developers may build something that breaks? – sisve Oct 10 '12 at 06:30
  • @Simon: Yes; I'm worried about someone accidentally injecting subtle bugs inside one of the template method's callbacks. I'm not worried about things that catastrophically explode; I'd assume devs wouldn't do that. – Billy ONeal Oct 10 '12 at 06:36

4 Answers4

4

You should make TemplateMethod's settings immutable:

abstract class TemplateMethod
{
    protected TemplateMethod(bool recursive)
    {
        Recursive = recursive;
    }

    public bool Recursive { get; private set; }
    protected abstract bool ItemIsWhitelisted(SomeType item);
    public IEnumerable<SomeType> GetItems() { /* ... */ }
}

class Implementer : TemplateMethod
{
    protected override bool ItemIsWhitelisted(SomeType item)
    {
        Recursive = false; // Oops; Recursive is read-only
        return item.CanFrobinate();
    }
}

UPD.

Option #2. If it is hard to pass settings via ctor, you may consider injection of immutable parameter object. Something like this (MEF-styled sample):

public interface ISettings
{
    bool Recursive { get; }
}

abstract class TemplateMethod
{
    [Import]
    public ISettings Settings { get; private set; }
    protected abstract bool ItemIsWhitelisted(SomeType item);
    public IEnumerable<SomeType> GetItems() { /* ... */ }
}

Of course, this means, that TemplateMethod can't change settings too.

Option #3. Explicit interface implementation (if TemplateMethod should be able to change settings):

public interface ISettingsWriter
{
    bool Recursive { set; }
}

abstract class TemplateMethod : ISettingsWriter
{
    public bool Recursive { get; private set; }

    bool ISettingsWriter.Recursive
    {
        set { Recursive = value; }
    }

    protected abstract bool ItemIsWhitelisted(SomeType item);
}

class Implementer : TemplateMethod
{
    protected override bool ItemIsWhitelisted(SomeType item)
    {
        Recursive = true; // Oops; Recursive is still read-only;

        return true;
    }
}

And of course, this means, that everyone, who wants to change TemplateMethod.Recursive, have to cast TemplateMethod to ISettingsWriter.

Dennis
  • 37,026
  • 10
  • 82
  • 150
  • Hmm.. that's interesting. +1. (But let's say in this case I can't do that) – Billy ONeal Oct 10 '12 at 06:43
  • @BillyONeal: OK. Should `TemplateMethod` itself be able to change these settings? – Dennis Oct 10 '12 at 06:46
  • Come to think of it; probably not. Perhaps I should have an "immutable settings block" or similar. – Billy ONeal Oct 10 '12 at 06:48
  • @BillyONeal what is the problem? Why can't you apply this solution? Seems exactly what you need to me. – jeroenh Oct 10 '12 at 06:51
  • @jeroenh: My example code is just that -- an example. I'm not sure this refactoring is going to be easier than refactoring to strategy in the real code. And really; my question is "does the language have a feature that does this"; and the fact that this is the answer seems to mean that the answer to my question is "no". But tis still a great answer, which is why it got +1 from me. – Billy ONeal Oct 10 '12 at 06:54
  • @BillyONeal: I've added a couple options to my answer, hope, this will be useful. – Dennis Oct 10 '12 at 07:29
1

I don't think you can reasonably represent such "callable by everyone, but not my child" restriction (child is one of everyone, and as can't be blocked without blocking everyone too...).

Not having such properties altogether is another approach (compared to making preoperties R/O in some way as suggested by @Dennis)

public IEnumerable<SomeType> GetItems(bool Recursive)...

or even

public IEnumerable<SomeType> GetItems(IterationSettings settings = DefaultSettings)

Note that such approach (similar to full strategy pattern for iteration) also solves the issue when external "trusted" (not child) caller changes properties in the middle of iteration (i.e. if there are some sort of callbacks/events fired by "child" implementation).

Alexei Levenkov
  • 98,904
  • 14
  • 127
  • 179
  • I think you're right. But figured the worst answer I could get by asking would be "no" :) – Billy ONeal Oct 10 '12 at 06:55
  • @BillyONeal, it interesting question so someone may find it useful later too. BTW, I've added note about "trusted" caller changing properties in the middle of iteration which should probably fail in some way (similar to "Collection modified" exception thrown by iterators when collection changed during iteration)... – Alexei Levenkov Oct 10 '12 at 07:03
1

If your "Framework" isn't published with source code, and the implementers are using a dll, you just can do this:

public abstract class TemplateMethod
{
    internal void DoSomething() { ... }
}

public abstract class Consumer
{
    TemplateMethod _Template = ...;
    protected void DoSomething()
    {
        _Template.DoSomething();
    }
}
Felix K.
  • 6,201
  • 2
  • 38
  • 71
0

You could try using stack trace to understand whether calling method may change the value or not.

    private bool _recursive;
    public bool Recursive {
        get
        {
            return _recursive;
        }
        set
        {
            StackTrace stackTrace = new StackTrace();
            if (stackTrace.GetFrame(1).GetMethod().Name == "ItemIsWhitelisted")
            {
                throw new Exception("Please don't");
            }
            _recursive = value;
        }
    }

or you could check DeclaringType of method in stack trace

stackTrace.GetFrame(1).GetMethod().DeclaringType
Dmitrii Dovgopolyi
  • 6,231
  • 2
  • 27
  • 44