1

Consider the following method...

private string[] GetExistingPOWSNumbers()
{
    using (
        var OleAdapter =
            new OleDbDataAdapter(
                "SELECT DISTINCT con_num FROM `Items` WHERE con_num LIKE 'POWS%'",
                ConfigurationManager.ConnectionStrings["MPOleConnectionString"].ConnectionString))
    using (var POWSTable = new DataTable())
    {
        OleAdapter.SelectCommand.Connection.Open();
        OleAdapter.Fill(POWSTable);
        return POWSTable.AsEnumerable().Select(row => Convert.ToString(row["con_num"])).ToArray();
    }
}

Are all the ADO.NET objects promptly being disposed of? I am using this method throughout my projects and when there are A LOT of calls like these being made during a single action, I receive "Out of Memory" errors.

EDIT: After some personal investigation I discovered that, in fact, the using statement for the adapter DOES NOT also close the provided collection. I made the following change. Now I am using a DataReader instead of populating a DataTable.

private string[] GetExistingPOWSNumbers()
{
    var Results = new List<string>();
    using (var OleConnection = new OleDbConnection(ConfigurationManager.ConnectionStrings["MPOleConnectionString"].ConnectionString))
    using (
        var OleCommand =
            new OleDbCommand(
                "SELECT DISTINCT con_num FROM `Items` WHERE con_num LIKE 'POWS%'",
                OleConnection))
    {
        OleConnection.Open();
        using (var OleReader = OleCommand.ExecuteReader(CommandBehavior.CloseConnection))
        {
            if (OleReader == null) return new string[0];
            while (OleReader.Read())
            {
                Results.Add(OleReader.GetString(0));
            }
        }
    }
    return Results.ToArray();
}
m-albert
  • 1,089
  • 8
  • 15
  • Your code seems to be a memory hog. In this particular case you should not have big problems but if your table is big and without a WHERE statement the problems could start to show. First you fill a DataTable then you call a Select on the same table copying every row in a second array that it contains the same data of the DataTable. While it is true that DataTable implenents IDisposable actually the Dispose method of the DataTable does nothing. http://stackoverflow.com/questions/913228/should-i-dispose-dataset-and-datatable – Steve May 09 '16 at 18:07
  • 1
    @Steve, I should probably be using a DataReader, yes? – m-albert May 09 '16 at 18:10
  • Well probably yes but this is not always practicable. It depends a lot on the size of your current code. If possible I would go for a review of your db calls to ensure that every step is taken to avoid loading unnecessary data (read WHERE) This sometimes requires a rework of the user interface and the effort required could be important. – Steve May 09 '16 at 18:12
  • My main concern is with the the OleDbDataAdapter. Does wrapping it in a using also close/dispose it's Command and Connection objects, or is it necessary to invoke those objects in using statements as well? – m-albert May 09 '16 at 18:12
  • @m-albert My answer says it will dispose of it. Unless your issue is that you return before disposing everything properly. If you are worried about that than assign it a string variable and return after the using statements are exited – DotNet Programmer May 09 '16 at 18:15
  • Uhm this code should not work according to MSDN on the OleDbDataAdapter. [The constructor that takes a connectionstring](https://msdn.microsoft.com/en-us/library/2f8y4737(v=vs.110).aspx) doesn't open the connection – Steve May 09 '16 at 18:16
  • Yeah... I recently changed the constructor to that one from the one that accepts a (new) select command object that was created with a new connection object. – m-albert May 09 '16 at 18:22
  • Looking at the [source code of the OleDbDataAdapter](http://referencesource.microsoft.com/#System.Data/System/Data/OleDb/OleDbDataAdapter.cs,e6eb930470a5a268) I can't see if the connection created by your constructor is disposed somewhere. To be on the safe side you could leave the actual using around the OleDbDataAdapter and avoid copying the rows but return directly the DataTable. – Steve May 09 '16 at 18:26
  • I need to return a string[] of con_num's that begin with "POWS", not a table. – m-albert May 09 '16 at 18:28
  • Have you tried my advice and assign it to a string[] and return it after the using closing }? – DotNet Programmer May 09 '16 at 18:43

1 Answers1

1

Objects will be cleaned up when they are no longer being used and when the garbage collector sees fit. Sometimes, you may need to set an object to null in order to make it go out of scope (such as a static field whose value you no longer need), but overall there is usually no need to set to null.

Regarding disposing objects, I agree with @Andre. If the object is IDisposable it is a good idea to dispose it when you no longer need it, especially if the object uses unmanaged resources. Not disposing unmanaged resources will lead to memory leaks.

You can use the using statement to automatically dispose an object once your program leaves the scope of the using statement.

using (MyIDisposableObject obj = new MyIDisposableObject())
{
    // use the object here
} // the object is disposed here

Which is functionally equivalent to:

MyIDisposableObject obj;
try
{
    obj = new MyIDisposableObject();
}
finally
{
    if (obj != null)
    {
        ((IDisposable)obj).Dispose();
    }
}

Got this from Zach Johnson here: Credit Due

Community
  • 1
  • 1
  • Great examples. I know that my using statement will dispose the adapter object. But will it also dispose the select command object and connection objects at the same time? – m-albert May 09 '16 at 18:24
  • @m-albert Then you should like him answer on the Credit Due page. It is a very good description of using disposing variables. – DotNet Programmer May 09 '16 at 19:03