4
\$\begingroup\$

I wrote an app that pulls a MS Access DB into a DataTable and then displays it into a WPF datagrid. The dialog window has a search button which calls on a class that does the DB work.

The methods in play of the search window:

private void ExitBtn_Click(object sender, MouseButtonEventArgs e)
{
    quoteheaders.Dispose();
    GC.Collect();
    Close();
}
private void TitleBar_DblClick(object sender, MouseButtonEventArgs e)
{
    if (this.WindowState == WindowState.Normal) { this.WindowState = WindowState.Maximized; return; }
    if (this.WindowState == WindowState.Maximized) { this.WindowState = WindowState.Normal; return; }
}
private void TitleBar_Click(object sender, MouseButtonEventArgs e)
{
    if (e.ChangedButton == MouseButton.Left) { DragMove(); }
}
private Task<DataTable> Poll_QuoteHeader()
{
    return Task.Run(() => SQLinterface.FindQuoteID(dbPath));
}
private async void SearchBtn_Click(object sender, RoutedEventArgs e)
{
    quoteheaders = SQLinterface.FindQuoteID(dbPath);
    quoteheaders = await Poll_QuoteHeader();
    foundQuotes.ItemsSource = quoteheaders.AsDataView();
}

The class that talks to the database:

namespace QuoteClient
{
    class SQLinterface
    {
    public DataTable FindServer()
    {
        DataTable SQLservers = new DataTable();
        SqlDataSourceEnumerator SQLinstances = SqlDataSourceEnumerator.Instance;
        SQLservers = SQLinstances.GetDataSources();
        return SQLservers; 
    }
    public DataTable FindQuoteID(string dbPath)
    {
        using(DataTable quoteID = new DataTable())
            {
            using (OleDbConnection quoteIDconn = new OleDbConnection(dbPath))
            {
                OleDbDataAdapter quoteIDadapter = new OleDbDataAdapter("SELECT * FROM DocumentHeaders", quoteIDconn);
                quoteIDadapter.Fill(quoteID);
                return quoteID;
            } 
        }
    }
}
}

I'm bringing this here because I would like to know if it is OK to use GC.Collect() in this case. The app works fine without forcing a collection, but increases in memory every time the window is opened and search button is clicked. I am undecided on whether this is an actual leak or just the GC taking its time to cleanup.

So in short is there a better way to ensure that the memory that is used for filling the data table is disposed of? (I dispose of them either via using or .Dispose() )

Should I just leave the collection call since I know that when this window is closed nothing that was in use is needed anymore?

Or am I doing something wrong all around?

\$\endgroup\$
  • 1
    \$\begingroup\$ Please use MVVM for developing WPF apps. You should avoid setting ItemsSource in code-behind. \$\endgroup\$ – Maxim Aug 13 '17 at 1:35
  • \$\begingroup\$ Duly noted, I just have not progressed to the point of MVVM. I'm still trying to experiment and understand WPF itself. Setting the ItemSource in code behind is not related to the memory usage I am seeing and learning how to handle. It happens when I do not show the data but only load it into a DataTable \$\endgroup\$ – Phexyaa Aug 13 '17 at 1:40
  • 1
    \$\begingroup\$ Are you sure this is working code? Why would you return a quoteID that you dispose with a using. This does not make any sense. \$\endgroup\$ – t3chb0t Aug 13 '17 at 7:38
  • \$\begingroup\$ It gets returned to another DataTable. It works. I suppose thats an improvement i can make to just use the first one, but i had trouble getting it to work properly with the asynchronous Task <DataTable> method \$\endgroup\$ – Phexyaa Aug 13 '17 at 7:51
  • 1
    \$\begingroup\$ As per this SO answer - calling Dispose doesn't actually do much for DataTable.. Also @t3chb0t is right: you dispose of the quoteID when you return from FindQuoteID - that's wrong. It happens to work because Dispose doesn't do much for DataTable but it's still wrong to do it. And you can't "free" memory in C#. The Dispose pattern in C# primarily exists to release unmanaged resources and not to release memory. There are very very few situations in which calling GC.Collect() might make sense but I don't think this is one of them. \$\endgroup\$ – ChrisWue Aug 13 '17 at 8:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.