CA2000 is a warning regarding the IDisposable interface:
CA2000 : Microsoft.Reliability : In method
'ImportProcessor.GetContext(string)', call System.IDisposable.Dispose
on object 'c' before all references to it are out of scope.
My method is used to store a cache of context like so:
public class RegionContext : IDisposable { /* Implement Dispose() here */ }
private Dictionary<string, RegionContext> contextCache = new ..... ();
public RegionContext GetContext(string regionCode)
{
RegionContext rc = null;
if (!this.contextCache.TryGetValue(regionCode.ToUpper(), out rc))
{
rc = new RegionContext(regionCode);
this.contextCache.Add(regionCode.ToUpper(), rc);
}
return rc;
}
Where would you use the using()
statement that fixes this compiler warning?
My outer class actually does iterate and dispose of the contents in the contextCache
in its own implementation. Shall I suppress it, or is there a way to correctly get rid of this warning?
This CA2000 warning comes up any time you have a return value that's IDisposable and don't handle the case where the method throws an exception. In that instance, the caller won't get a valid instance of your object, so it has no way to dispose it. Therefore you have to.
I assume that you won't want to dispose the object if you pull it out of your cache successfully. In that case, you would need to do something like this to make sure the object you might create locally gets disposed in all cases:
public RegionContext GetContext(string regionCode)
{
RegionContext temp = null;
RegionContext rc = null;
try
{
if (!this.contextCache.TryGetValue(regionCode.ToUpper(), out rc))
{
temp = new RegionContext(regionCode);
this.contextCache.Add(regionCode.ToUpper(), temp);
rc = temp;
temp = null;
}
return rc;
}
finally
{
if ( temp != null )
{
temp.Dispose();
}
}
}
What CA2000 is complaining about here is that the variable could be "orphaned" in an undisposed state if there's an exception while attempting to add it to the cache. To address the problem thoroughly, you could add a try/catch as follows (the newContext
variable is used only so that CA2000 can detect the fix):
public RegionContext GetContext(string regionCode)
{
RegionContext rc = null;
if (!this.contextCache.TryGetValue(regionCode.ToUpper(), out rc))
{
RegionContext newContext = new RegionContext(regionCode);
try
{
this.contextCache.Add(regionCode.ToUpper(), newContext);
}
catch
{
newContext.Dispose();
throw;
}
rc = newContext;
}
return rc;
}
Personally, I find this sort of thing to be somewhat ridiculous overkill in most cases, but ymmv...
Michael's solution does not seem to work when converted to VB.Net. The following two functions were tested under VS 2017:
Public Function OpenStream(ByVal filePathName As String) As System.IO.FileStream
Dim fileStream As System.IO.FileStream = Nothing
Dim tempFileStream As System.IO.FileStream = Nothing
If Not String.IsNullOrWhiteSpace(filePathName) Then
Try
tempFileStream = New System.IO.FileStream(filePathName, System.IO.FileMode.Open, System.IO.FileAccess.Read)
fileStream = tempFileStream
Catch
tempFileStream?.Dispose()
Throw
End Try
End If
Return fileStream
End Function
Public Function OpenReader(ByVal filePathName As String) As System.IO.BinaryReader
If String.IsNullOrWhiteSpace(filePathName) Then Throw New ArgumentNullException(NameOf(filePathName))
If Not System.IO.File.Exists(filePathName) Then Throw New System.IO.FileNotFoundException("Failed opening a binary reader -- file not found.", filePathName)
Dim tempReader As System.IO.BinaryReader = Nothing
Dim reader As System.IO.BinaryReader = Nothing
Dim stream As IO.FileStream = Nothing
Try
stream = Methods.OpenStream(filePathName)
tempReader = New System.IO.BinaryReader(stream)
reader = tempReader
Catch
stream?.Dispose()
tempReader?.Dispose()
Throw
End Try
Return reader
End Function