Narrowed question:
I need to implement reusable functionality that initializes a temporary file, runs some custom logic provided by the user, and then deletes the file. I can do this either through a static utility method that takes the user's logic as a delegate, or through a class that implements IDisposable
to perform cleanup during Dispose
. The two approaches would be used as follows:
// Delegate-parameterized method approach:
TempFile.Using(filePath =>
{
// use temporary file here
});
// IDisposable implementation approach:
using (var tempFile = new TempFile())
{
// use temporary file here
}
Which are the advantages and drawbacks to each approach? Which one should I use?
Original question:
I am developing some general-purpose functionalities that require a fixed “initialization – custom logic – clean-up” sequence. The clean-up must be performed as part of the same construct used for executing the operation; I don't want to confer the responsibility for calling a Cleanup
or Close
method to the user.
For example, I might want to provide functionality that automatically creates and deletes a temporary file. The simplest way to implement this is through a method that takes an Action<T>
delegate:
public static void UsingTempFile(Action<string> action)
{
// initialization
string tempFilePath = Path.GetTempFileName();
try
{
action(tempFilePath);
}
finally
{
// clean-up
File.Delete(tempFilePath);
}
}
This could be used like so:
UsingTempFile(filePath =>
{
File.WriteAllText(filePath, "Hello world");
string text = File.ReadAllText(filePath);
});
However, this approach often requires me to implement four method overloads to support returned results and anonymous functions:
public static void UsingTempFile(Action<string> action) { /* ... */ }
public static TResult UsingTempFile<TResult>(Func<string, TResult> func) { /* ... */ }
public static async Task UsingTempFile(Func<string, Task> asyncAction) { /* ... */ }
public static async Task<TResult> UsingTempFile<TResult>(Func<string, Task<TResult>> asyncFunc) { /* ... */ }
The former three can implemented as simple wrappers that call the last overload. However, in a public API, they still need to be documented and unit-tested, substantially cluttering my codebase.
An alternative approach is to design an instantiatable class that implements IDisposable
to represent the operation. This class would perform the initialization in its constructor, and the clean-up in its Dispose
method. It could then be consumed like so:
using (var tempFile = new TempFile())
{
File.WriteAllText(tempFile.FilePath, "Hello world");
string text = File.ReadAllText(tempFile.FilePath);
}
The advantage of this approach is that the C# compiler automatically handles all my four cases ‒ the user can specify return
and await
keywords in the using
block.
However, I often need to be able to throw exceptions from my clean-up logic – like IOException
being thrown from File.Delete
if the temporary file is still in use in the above example. The MSDN documentation for the Dispose Pattern states:
X AVOID throwing an exception from within
Dispose(bool)
except under critical situations where the containing process has been corrupted (leaks, inconsistent shared state, etc.).Users expect that a call to
Dispose
will not raise an exception.If
Dispose
could raise an exception, further finally-block cleanup logic will not execute. To work around this, the user would need to wrap every call toDispose
(within the finally block!) in a try block, which leads to very complex cleanup handlers.
A related question makes a stronger case about the downsides to throwing exceptions from finally
blocks:
- A first exception is thrown
- A finally block is executed as a result of the first exception
- The finally block calls a Dispose() method
- The Dispose() method throws a second exception
[…] You lose information because .NET unceremoneously replaces the first exception with the second one. A catch block somewhere up the call stack will therefore never see the first exception. However, one is usually more interested in the first exception because that normally gives better clues as to why things started to go wrong.
This argument has merit – if the user's custom logic throws an exception, I would not want it to be hidden (and lost) by any exceptions thrown in the clean-up. There are solutions to this, such as Marc Gravell's wrapper for IDisposable
objects (which swallows all Dispose
exceptions), but this imparts more responsibility on the user. On the other hand, the .NET Framework Class Library itself seems to disregard this rule ‒ FileStream
can throw exceptions from its Dispose
method if it fails to flush the buffer's contents to disk.
Which of the two approaches (delegate-parameterized methods vs IDisposable
) would be recommendable for these general-purpose implementations? Are there any implications apart from the ones mentioned above?
Update: This is an example of how delegate-parameterized methods can prevent main-logic exceptions from being swallowed:
public static void UsingTempFile(Action<string> action)
{
// initialization
string tempFilePath = Path.GetTempFileName();
bool isSuccess = false;
try
{
// main logic
action(tempFilePath);
isSuccess = true;
}
finally
{
try
{
// clean-up
File.Delete(tempFilePath);
}
catch
{
// only propagate exceptions from clean-up if there were
// no unhandled exceptions from the main logic
if (isSuccess)
throw;
}
}
}