I have an abstract base class from which many classes are derived. I want derived classes to be able to override a virtual method defined in the base class, but there is complex logic in the base class that determines whether t开发者_如何学运维he overridden method is "enabled" at any particular moment.
Consider this code -- one possible solution -- for example:
public abstract class AbstractBaseClass
{
public bool IsMethodEnabled { get; set; }
public virtual void DerivedMethod() { }
public void Method()
{
if (IsMethodEnabled)
DerivedMethod();
}
}
public class DerivedClass : AbstractBaseClass
{
public override void DerivedMethod()
{
Console.WriteLine("DerivedMethod() was called.");
}
}
In the example above, IsMethodEnabled
is shorthand for more complex logic that determines whether DerivedMethod
should be called -- it's code that I want encapsulated in the base class so that I don't have to reproduce it in each derived class.
The design works as intended. If I run this sample code:
AbstractBaseClass a1 = new DerivedClass() { IsMethodEnabled = false };
AbstractBaseClass a2 = new DerivedClass() { IsMethodEnabled = true };
a1.Method();
a2.Method();
...I see exactly one call to DerivedMethod
, as expected.
But something rubs me wrong about this implementation. I feel like there must be a more elegant way to handle this. Is there a better way to selectively call a derived class's method implementation from its abstract base class? Is there a design pattern that would better serve me here?
In other words, does the code above "smell"?
This is a perfectly reasonable implementation.
The main changes I would suggest are:
- Make the virtual method that implements the functionality
protected
instead of public - Use more appropriate naming for this. Perhaps something more like
public void Method()
andprotected virtual void OnMethod()
I agree with Reed that this is a reasonable implementation.
However, I'd consider the following: who are you trying to protect here? I am all for designing base classes well so that they can be extended easily and safely, but I'm also of the opinion that the developer writing the derived class knows more than the developer who wrote the base class. They might know better than you do whether a given method is "enabled" or not.
It does not 'smell' more than other Template Methods, which are not liked by some people. I tend to agree with some points made here. Especially these two:
Difficult to comprehend program flow – In my experience it takes very few levels of template methods and inheritance to make debugging or understand the sequence of method calls difficult (as few as 2 or 3). When template methods are really pushed (lots of abstract methods at multiple levels), it can become painful to debug this kind of a system.
Difficult to maintain – Having maintained a couple chunks of code that made extensive use of the template method, it can be challenging. This kind of system can rapidly become fragile. Changes at any one level can disturb operation above or below that level in the template methods. There is often a feeling of unpredictability when adding new functionality as it difficult to predict how behavior will change in all cases. You often also tend to build finer and finer tweaks by splitting the algorithmic parts of the template class and inserting more layers, thus exacerbating the problem.
Generally speaking I think you have to be very careful with Template Method and keep things simple and focused.
It seems you're trying to decouple the decision about calling a method from the method itself. If the sole reason to have the base class is to encapsulate that decision, and make its code reusable, I think you could use a more loosely-coupled design, which would ease testing each behavior separately:
public interface IDoSomething {
void Method();
}
public class ConditionallyDoSomething : IDoSomething {
private IDoSomething _wrapped;
public ConditionallyDoSomething(IDoSomething wrapped) {
_wrapped = wrapped;
}
public bool IsMethodEnabled { get; set; } // could be quite complex...
public void Method() {
if (IsMethodEnabled) {
_wrapped.Method();
}
}
}
public class DoSomething : IDoSomething {
public void Method() {
// do something...
}
}
This way, you can mock IDoSomething
s and test each piece (decision making and functionality) separately. But this is only warranted if you really have some complex logic in both behaviors that would benefit from such separation. I'm just trying to give an alternative to the other excellent answers here. Ultimately, it depends on your specific scenario.
You could just mark all methods that are required to be overriden as abstract and those methods that could be optionally overriden as virtual. See C# Abstract Classes
It depends on how expensive IsMethodEnabled (really) is, assuming it isn't the compiler generated one as shown, and whether IsMethodEnabled is going to change frequently, and whether there are hundreds of methods with that little "is enabled" bit of logic there, and whether Method() is a really performance critical path.
精彩评论