According to Uncle Bob's principles, most of which I agree with, passing Boolean parameters is something to be avoided. To understand why you first need to agree with the Single Responsibility Principle (SRP).
The Single Responsibility Principle (SRP) states that there should be one and only one reason for a class to change. It follows therefore that methods within such a class should perform one responsibility only.
Readers should note that this is a Principle (guideline) and not a Rule. This distinction is often not made by those who choose to criticize this principle. It follows that the only appropriate criticism for a Principle, is an opposing principle and not one or a set of edge cases where logic may lead one to overrule a Principle. A suitable argument against SRP would be along the lines of ... ' a class does not deserve to be a class unless it has more than one responsibility' - a Multi-Responsibility Principle (MRP) if you will. It certainly cannot follow that as a Principle that both SRP classes and MRP classes are equally correct.
If after some consideration you decide to agree and abide by this principle you will find that it is improbable that you could build a method that receives a boolean parameter that does not violate this principle. For example
CALL DoThis (WithThisData, WithThisBoolean)...
PROCEDURE DoThis (WithThisData, WithThisCondition)
BEGIN
IF (WithThisConditionIsTrue)
<do some stuff which amounts to responsibility1 WithThisData>
ELSE
<do some stuff which amounts to responsibility2 WithThisData>
END
Instead the pseudocode above should be ...
IF (WithThisConditionIsTrue)
DoResponsibility1 (WithThisData)
ELSE
DoResponsibility2 (WithThisData)
PROCEDURE DoResponsibility1 (WithThisData)
...
PROCEDURE DoResponsibility2 (WithThisData)
....
Uncle Bob writes ...
F3: Flag ArgumentsAnother reason why I like this principle is that according to my philosophy on the reasons why we write code (summarized by the simple answer: so that others can read it!) is that it is almost always harder to understand the intent of a function that receives a flag (boolean) argument.
Boolean arguments loudly declare that the function does more than one thing. They are confusing and should be eliminated.
It follows therefore that our well published C# IDisposable pattern is not SOLID code because it uses a boolean argument. The pattern is not as widely used as it should be and many people incorrectly publish that the pattern does nothing useful. I blame part of the confusion and differing opinions on this one tiny flag passed into the Dispose(true) call.
I wont go into explaining the pattern itself, but let me explain that it looks like this...
public class BothType: IDisposable{
public void Dispose(){
Dispose(true);
GC.SupressFinalize(this);
}
~BothType(){ //finalizer
Dispose(false);
}
protected virtual void Dispose(bool disposing){
if(disposing){
//'Disposable' managed resource cleanup code
}
//Unmanaged resource cleanup code
base.Dispose(disposing);
}
Managed code developers (Java, Dot.Net, PHP, Python, Ruby etc) are abstracted from most memory and resource management concerns, and in most applications ignoring even the attention required by the small number of unmanaged resource that are allocated and never properly released is uneventful because our modern machinery is grossly over-spec. But this behavior fails quickly in a stressed server environment where resources are always under pressure.
The pattern above ensures that if the code did not dispose of unmanaged resources by the time the wrapping managed code is Garbage Collected then at least the finalizer frees the unmanaged resources. Of all the explanations that can be found online, most people do not focus on this above reason which is the reason for the pattern to exist in the first place. You will find all manner of interesting chatter, from the fact that Dispose is not called by the Garbage Collector to the fact that the presence of a Finalizer causes your class to survive at least one Garbage Collection attempt (usually followed by the incorrect advise to never create the finalizer). But not many people focus on this one reason why the Dispose pattern exists – which is to attempt to free unmanaged resources before Garbage Collection takes place or to at least catch unreleased resources at the last possible moment and release them then.
The correct pattern does this:
- If your class encapsulates unmanaged resources it must ensure that these resources are released by calling Dispose(). The Dot.Net framework standard method to accomplish this is the IDiposable interface.
- It follows that Disposing of a classes resources should cause that class to call Dispose on all its encapsulated classes and its base class as well if these implement IDisposable.
- You (the developer) must ensure IDisposable.Dispose is called when the class is no longer needed.
- Dispose must be allow itself to be called multiple times.
- Dispose must release the classes own UnManaged resources.
- Dispose must then suppress the classes Finalizer by calling GC.SupressFinalize(this)
- The Finalizer should contain code that does nothing more than release the classes own unmanaged resources. It is not expected to be called since Dispose should suppress its calling. However, it is a belt and braces approach, defensive coding if you will.
Now, how much of the above did you get from the code snippet? Unfortunately the pattern is setup as is and its too late to change its implementation. The Dispose(boolean disposing) method is meant to be inherited, virtually overridden and called by base.Dispose(disposing) to chain the release of resource request down to parent classes.
A pity. I think the following communicates the code intent in a much better fashion...
#region IDiposable
/// <summary>
/// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
/// </summary>
public void Dispose()
{
DisposeManaged();
DisposeUnManaged();
GC.SuppressFinalize(this);
}
/// <summary>
/// Calls Dispose() on all our managed resources
/// </summary>
private void DisposeManaged()
{
this.SomeClassA.Dispose();
this.SomeClassB.Dispose();
}
/// <summary>
/// Disposes our unmanaged resources
/// </summary>
private void DisposeUnManaged()
{
this._managedResourceA.Close();
this._managedResourceB.Free();
}
~ABetterDisposerPattern()
{
DisposeUnManaged();
}
#endregion IDisposable
The above accomplishes everything that the traditional IDispose pattern accomplishes - and look! It does it with clarity of intent and far easier legibility, don't you think?.
0 comments:
Post a Comment