Code weave helper for the standard Dispose pattern?
I have been reading Effective C# and a few other such books/blogs recently and when talking about the standard Dispose pattern (which I'm already using) they all recommend using the class' dispose variable (as defined in that MSDN sample code) at the beginning of every method. Essentially to insure that once Dispose has been called, any attempt to use the object would result in ObjectDisposedException. This makes sense, but is an enormous amount of manual labor in a large enough code base and relies on humans remembering to do it. So I am looking for a better way.
I recently came across and started using the notifypropertyweaver that automatically fills out all the boilerplate code of calling the PropertyChanged handlers (works as an msbuild task, thus requiring no additional shipping dependency). I wonder if anyone knows of a similar solution for the standard dispose pattern. What it would essentially do is accept a variable name as config (the bool disposed in MSDN's sample case), and add the following code to every method that isn't the Finalizer or named Dispose in every class that implements IDisposable:
if(disposed)
throw new ObjectDisposedException();
Does such a thing exist? Alternatively what do people do to achieve this in their code, manually add the if-statement?
Clarification of Purpose
The greater need for this is not some "best practice" drive, but that we do have users managing the life-cycles of our objects improperly. Right now they just get a NullReference or some other such thing from underlying resource, which could mean we have a bug in our library, I want to communicate to them that they are the ones creating the issue and how they are crating it (considering I'm in position to know). So suggestions that the users of our Types are the ones who should be taking care of this, isn't really productive here.
Update: My original response didn't really answer the question so here is another attempt...
To help solve the root problem, developers forgetting to throw ObjectDisposedExceptions , perhaps automated unit testing will do the trick. If you want to be strict about requiring all methods/properties to throw ObjectDisposedException immediately if Dispose has already been called then you can use the following unit test. Just specify the assemblies you want to test. You will probably need to modify the IsExcluded method as necessary and the object mocking may not work in all cases.
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using MbUnit.Framework;
using Moq;
[TestFixture]
public class IDisposableTests
{
[Test]
public void ThrowsObjectDisposedExceptions()
{
var assemblyToTest = Assembly.LoadWithPartialName("MyAssembly");
// Get all types that implement IDisposable
var disposableTypes =
from type in assemblyToTest.GetTypes()
where type.GetInterface(typeof(IDisposable).FullName) != null
select type;
foreach (var type in disposableTypes)
{
// Try to get default constructor first...
var constructor = type.GetConstructor(Type.EmptyTypes);
if (constructor == null)
{
// Otherwise get first parameter based constructor...
var constructors = type.GetConstructors();
if (constructors != null &&
constructors.Length > 0)
{
constructor = constructors[0];
}
}
// If there is a public constructor...
if (constructor != null)
{
object instance = Activator.CreateInstance(type, GetDefaultArguments(constructor));
(instance as IDisposable).Dispose();
foreach (var method in type.GetMethods())
{
if (!this.IsExcluded(method))
{
bool threwObjectDisposedException = false;
try
{
method.Invoke(instance, GetDefaultArguments(method));
}
catch (TargetInvocationException ex)
{
if (ex.InnerException.GetType() == typeof(ObjectDisposedException))
{
threwObjectDisposedException = true;
}
}
Assert.IsTrue(threwObjectDisposedException);
}
}
}
}
}
private bool IsExcluded(MethodInfo method)
{
// May want to include ToString, GetHashCode.
// Doesn't handle checking overloads which would take more
// logic to compare parameters etc.
if (method.Name == "Dispose" ||
method.Name == "GetType")
{
return true;
}
return false;
}
private object[] GetDefaultArguments(MethodBase method)
{
var arguments = new List<object>();
foreach (var parameter in method.GetParameters())
{
var type = parameter.ParameterType;
if (type.IsValueType)
{
arguments.Add(Activator.CreateInstance(type));
}
else if (!type.IsSealed)
{
dynamic mock = Activator.CreateInstance(typeof(Mock<>).MakeGenericType(type));
arguments.Add(mock.Object);
}
else
{
arguments.Add(null);
}
}
return arguments.ToArray();
}
}
Original Response : It doesn't look like there is anything like NotifyPropertyWeaver for IDisposable so if you want that you'll need to create a similar project yourself. You can potentially save yourself a little work by having a base Disposable class like in this blog entry. Then you just have a one-liner at the top of each method: ThrowExceptionIfDisposed() .
However, neither possible solution feel right or seem necessary. In general throwing ObjectDisposedException is not needed that often. I did a quick search in Reflector and the ObjectDisposedException is thrown directly by just 6 types in the BCL, and for a sample outside the BCL System.Windows.Forms only has one type that throws: Cursor when getting the Handle.
Basically, you only need to throw ObjectDisposedException if a call on your object will fail specifically because Dispose was already called, such as if you set some field to null that is needed by a method or property. An ObjectDisposedException will be more informative than a random NullReferenceException , but unless you're cleaning up unmanaged resources it's not usually necessary to set a bunch of fields to null. Most of the time if you're just calling Dispose on other objects, it's up to them to throw ObjectDisposedException .
Here is a trivial example you might throw ObjectDisposedException explicitly:
public class ThrowObjectDisposedExplicity : IDisposable
{
private MemoryStream stream;
public ThrowObjectDisposedExplicity()
{
this.stream = new MemoryStream();
}
public void DoSomething()
{
if (this.stream == null)
{
throw new ObjectDisposedException(null);
}
this.stream.ReadByte();
}
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
if (this.stream != null)
{
this.stream.Dispose();
this.stream = null;
}
}
}
public void Dispose()
{
this.Dispose(true);
GC.SuppressFinalize(this);
}
}
With the code above though there is really no need set stream to null. You could just rely on the fact that MemoryStream.ReadByte() will throw ObjectDisposedException on it's own like the code below:
public class ThrowObjectDisposedImplicitly : IDisposable
{
private MemoryStream stream;
public ThrowObjectDisposedImplicitly()
{
this.stream = new MemoryStream();
}
public void DoSomething()
{
// This will throw ObjectDisposedException as necessary
this.stream.ReadByte();
}
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
this.stream.Dispose();
}
}
public void Dispose()
{
this.Dispose(true);
GC.SuppressFinalize(this);
}
}
The first strategy of setting stream to null may make sense in some cases, such as if you know the object will throw an exception if Dispose is called multiple times. In that case you want to be defensive and make sure your class doesn't throw an exception on multiple calls to Dispose . Besides that I can't think of any other cases off hand where you would need to set fields to null which would probably require throwing ObjectDisposedExceptions .
Throwing an ObjectDisposedException isn't needed often and should be considered carefully, so the code weaving tool you desire probably isn't necessary. Use Microsoft's libraries as your example, see which methods that implement actually throw ObjectDisposedException when the type implements IDisposable .
Note: GC.SuppressFinalize(this); isn't strictly needed since there is no finalizer, but it's left there since a subclass may implement a finalizer. If the class was marked as sealed then GC.SupressFinalize could be safely removed.
Instead of using the NotifyPropertyWeaver, I suggest that you solve your problem another way. You claim to have two issues:
In order to defray the cost of re-writing very similar code, I suggest that you create and use code snippets in Visual Studio. The problem that snippets seek to solve is exactly #2 in the list above. See this MSDN article for instructions.
The problem with #1 in the list above is that the IDisposable pattern is not always necessary on 100% of your classes. That makes it difficult for a static analysis to "catch" whether or not a given class ought to be disposable. (edit: After thinking about it for a minute, it actually wouldn't be that difficult to check. If your current class contains instances that are IDisposable, your class should be IDisposable)
Because of this, I recommend that you use a Code Review in order to catch those cases where a developer should make their class disposable. You can add it to a checklist of items that the developer should self-check before asking for a code review.
链接地址: http://www.djcxy.com/p/57278.html下一篇: 代码编织助手的标准配置模式？