CODEWITHSUNDEEP
c#c#-4.0
Michael Goldshteyn
Michael Goldshteyn

Reputation: 74370

using guidelines for generics and IDisposable

In item 5 of More Effective C#, the following is presented:

public class EngineDriver<T> where T : IEngine, new()
{
  public void GetThingsDone()
  {
    T driver = new T();
    using (driver as IDisposable)
    {
      driver.DoWork();
    }
  }
}

The goal here is to properly dispose of drive, if it implements IDisposable. This makes sense, but how is this implementation different from the more succinct:

public class EngineDriver<T> where T : IEngine, new()
{
  public void GetThingsDone()
  {
    using (T driver = new T())
    {
      driver.DoWork();
    }
  }
}

Shouldn't the code above behave in exactly the same way? In fact, isn't the original code dangerous in that the lifetime of driver is beyond the using block, but driver is disposed of at the end of said block?

Upvotes: 5

Views: 1241

Answers (3)

Jord&#227;o
Jord&#227;o

Reputation: 56467

Your second proposal will only work if IEngine implements IDisposable or if an extra constraint (for IDisposable) is added to the type parameter on EngineDriver. The original code is flexible enough to additionaly handle IEngine implementations that also implement IDisposable.

If you're really worried about using the object after it's been disposed of, you could create another scope to encompass the variable:

public class EngineDriver<T> where T : IEngine, new() {
  public void GetThingsDone() {
    {
      T driver = new T();
      using (driver as IDisposable) {
        driver.DoWork();
      }
    }
  }
}

But for this example, this is overkill; the scope of the method is enough. It would only make sense if your method was bigger, like:

public class EngineDriver<T> where T : IEngine, new() {
  public void GetThingsDone() {
    {
      T driver = new T();
      using (driver as IDisposable) {
        driver.DoWork();
      }
    }
    // do more stuff, can't access driver here ....
  }
}

But then again, I'm just showing what's possible here, I'd prefer to refactor it so that that block is isolated from the other code.

You can also have another scoping class:

public class DisposableWrapper<T> : IDisposable {
  public T Item { get; private set; }
  public DisposableWrapper(T item) { Item = item; }
  public void Dispose() {
    using (Item as IDisposable) { }
    Item = default(T);
  }
}

public static class DisposableWrapperExtensions {
  public static DisposableWrapper<T> AsDisposable<T>(this T item) {
    return new DisposableWrapper<T>(item);
  }
}

public class EngineDriver<T> where T : IEngine, new() {
  public void GetThingsDone() {
    using (var driver = new T().AsDisposable()) {
      driver.Item.DoWork();
    }
  }
}

It might make sense if you use many interface references that could implement IDisposable.

Upvotes: 1

Lee
Lee

Reputation: 144136

No, because T does not necessarily implement IDisposable (unless IEngine itself implements it) - the second will not compile in this case, whereas the first will.

Regarding the scope of driver - it will still be accessible after the using block in the second example which is not ideal, and attempting to do so would usually result in an exception. Ideally you should have IEngine implement IDisposable or add an additional constraint to EngineDriver that T implement it.

Upvotes: 10

Hans Passant
Hans Passant

Reputation: 941455

It is kinda important that you use the tools that are available to you. Compile your proposed code. I'll wait a few minutes.

Okay, you're back. Yes, you need to add a constraint for IDisposable so that the using statement is always capable of disposing the object. The code from the book is a hack around that restriction, it will work even if T does not implement IDisposable. using (null) {} is valid.

Upvotes: 7

Related Questions