0

I'm working on a Windows Service under C# which basically scans a specific directory for newly created files. If a file has been created, the filename string is split and a specific substring of the filename used to query a SQL database. As this service is running permanently i want it to be as memory friendly as possible and therefore I've been told to use the automatic garbage collection while putting the SqlDataAdapter and the DataTable objects in usings. Take the following code into consideration:

using (SqlDataAdapter da = new SqlDataAdapter(cmdstring, con))
{
    using (DataTable dt = new DataTable())
    {
        da.Fill(dt);

        if (dt.Rows.Count != 0)
        {
            splitlst.Add(dt.Rows[0][0].ToString());//kunde
            splitlst.Add(dt.Rows[0][1].ToString());
            splitlst.Add(dt.Rows[0][2].ToString());
            splitlst.Add(dt.Rows[0][3].ToString());
            splitlst.Add(dt.Rows[0][4].ToString());
            splitlst.Add(dt.Rows[0][5].ToString());
        }
    }
}

Is this an efficient way to handle the garbage collection? I'm not really used to working this way, so if anyone could make me understand when to deliberately control the garbage collection, i would be very greatful.

mjwills
  • 23,389
  • 6
  • 40
  • 63
Mondblut
  • 199
  • 1
  • 14
  • Generally, if you're interacting with processes/services/programs/whatever that aren't part of your application, a using statement is a good idea. – Danny Goodall Apr 09 '18 at 12:48
  • 3
    `using` is to do with `IDisposable` not `garbage collection`. – mjwills Apr 09 '18 at 12:48
  • 5
    You don't control the garbage collection here. Use the `using`-statement whenever possible(`IDisposable`). But that has nothing to do with the garbage collector which is responsible for collecting garbage(memory occupied by objects that are no longer in use). Disposing is about unmanaged objects and garbage collection about managed objects. – Tim Schmelter Apr 09 '18 at 12:49
  • Possible duplicate of [using statement - does it trigger garbage collection?](https://stackoverflow.com/questions/11927827/using-statement-does-it-trigger-garbage-collection) – mjwills Apr 09 '18 at 12:51
  • I see, so it was a missunderstanding from my side. But either way, is the code above advisable? The SqlDataAdapter object and the DataTable object are being disposed each time after they're used, is this assumption correct? – Mondblut Apr 09 '18 at 12:52
  • Yes that is correct. – mjwills Apr 09 '18 at 12:53
  • Any object that supports disposal should be disposed as soon as possible. You don't need to nest `using` blocks unless you need to execute code between them. Otherwise, just put all the `using` statements at the top of one block. – jmcilhinney Apr 09 '18 at 12:53
  • 2
    Whenver you create an `IDisposable` object, it's always a good practice to call `Dispose` on it as soon as possible. `using` is a good way to do that if that's going to happen within the same method. – Alejandro Apr 09 '18 at 12:54
  • Also, don't use a data adapter and a `DataTable` at all in that case. Use a data reader instead. It can be optimised for a one-record result set too. – jmcilhinney Apr 09 '18 at 12:54
  • Another related question: Using my code above in debugging I see that the garbage collection pointers being shown on the graph constantly even though the application is basically in idle mode or rather waiting for the event to be executed again. Is this because i used the usings for the disposal of the SqlDataAdapter object before? – Mondblut Apr 09 '18 at 13:18
  • I would suggest writing a new question for that @Mondblut. – mjwills Apr 09 '18 at 21:47

1 Answers1

1

The code is good; but for the wrong reasons (sort of).


I've been told to use the automatic garbage collection

Somewhat facetiously, you don't need to explicitly use the automatic garbage collection. Because it's automatic.


using blocks are used for disposing objects (which implement the IDisposable interface).

The intention (lowering memory usage) is the same here. Many disposable objects free their data; so that the automatic garbage collector is allowed to clean it when it wants to.


As a very minor comment, you can reduce nesting by omitting the brackets of the top using statement:

using (SqlDataAdapter da = new SqlDataAdapter(cmdstring, con))
using (DataTable dt = new DataTable())
{
    da.Fill(dt);

    if (dt.Rows.Count != 0)
    {
        splitlst.Add(dt.Rows[0][0].ToString());//kunde
        splitlst.Add(dt.Rows[0][1].ToString());
        splitlst.Add(dt.Rows[0][2].ToString());
        splitlst.Add(dt.Rows[0][3].ToString());
        splitlst.Add(dt.Rows[0][4].ToString());
        splitlst.Add(dt.Rows[0][5].ToString());
    }
}

Similar to other code blocks (if, foreach, ...), the braces are not necessary for a single line of code.

Technically, you'd still need to indent. Similar to doing:

if(1 + 1 != 2) 
    BurnMathBooks(); //still indented

You should technically also do:

using (SqlDataAdapter da = new SqlDataAdapter(cmdstring, con))
    using (DataTable dt = new DataTable())  //still indented
    {
        //...
    }

But since the lack of indentation doesn't actually make the code more unreadable; it's acceptable to omit it in this case.

using (SqlDataAdapter da = new SqlDataAdapter(cmdstring, con))
using (DataTable dt = new DataTable())
{
    //...
}
Flater
  • 12,908
  • 4
  • 39
  • 62