1

Below is my entire class that I am using, I have two questions, 1 is this the proper use of Dispose() and also, why am I getting the error No Overload for method 'dispose' takes 1 argument.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Crawler
{
    class LoggingClass : IDisposable
    {
        public void GenericLogging(string systemMsg, string SystemClass, string SystemSection, string ID, string FixID, string baseURL, string mysqlQueryName, string mysqlQuery)
        {
            string Loggingall = " insert into tblLogs " +
                            "set SystemMsg='" + systemMsg.Replace("'","''") + "'" +
                            ",SystemClass = '" + SystemClass.Replace("'", "''") + "'" +
                            ",SystemSection = '" + SystemSection.Replace("'", "''") + "'" +
                            ",ID = '" + CarID.Replace("'", "''") + "'" +
                            ",FixID = '" + FixID.Replace("'", "''") + "'" +
                            ",baseurl = '" + baseURL.Replace("'", "''") + "'" +
                            ",mysqlqueryName = '" + mysqlQuery.Replace("'", "''") + "'" +
                            ",mysqlquery = '" + mysqlQuery.Replace("'", "''") + "'" +
                            ",TimeStamp = Now()";
            MySQLProcessing.MySQLProcessor MYSQLP = new MySQLProcessing.MySQLProcessor();
            MYSQLP.MySQLInsertUpdate(Loggingall, "Loggingall");
        }

        public void Dispose()
        {
            Dispose(true);
            // Take yourself off the Finalization queue 
            // to prevent finalization code for this object
            // from executing a second time.
            GC.SuppressFinalize(this);
        }
    }
}

Here is my updated code: }

Is this the correct way to call it? Do i also have to call the dispose?

2
  • 2
    Is there more to this class than you're showing here? The class as it is does not need to implement IDisposable at all.
    – LukeH
    Commented Apr 5, 2012 at 23:46
  • Similar: stackoverflow.com/q/9472304/1026459
    – Travis J
    Commented Apr 5, 2012 at 23:58

3 Answers 3

6

As other answerers have mentioned, it doesn't look like you need to implement IDisposable. You have no class fields at all, so there's nothing to clean up.

Assuming there's more to the class than you've shown, you're following the pattern for implementing IDisposable, but you're only halfway done.

The pattern is to have IDisposable.Dispose() and the finalizer (~LoggingClass) both call a common method, Dispose(bool). In the Dispose(bool) method, you should clean up both managed and unmanaged resources if the boolean is passed true, and only clean up the unmanaged resources if passed false.

Here's the code I use for implementing IDisposable.

~LoggingClass()
{
    this.Dispose(false);
}

protected bool Disposed { get; private set; }

public void Dispose()
{
    this.Dispose(true);
    GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
    if (!this.Disposed)
    {
        if (disposing)
        {
            // Perform managed cleanup here.

        }

        // Perform unmanaged cleanup here.

        this.Disposed = true;
    }
}

Edit

It looks like you added your updated code, and then removed it. But, here's my comments on how you're calling it.

With the GenericLogging method as you currently have it, you don't need IDisposable at all. However, there are a couple things I would do to improve up your code.

  1. Create the MySQLProcessing.MySQLProcessor instance in the constructor, rather than in the GenericLogging method.
  2. Call MySQLProcessing.MySQLProcessor.Dispose() (or .Close(), or whatever that class has) in the managed cleanup section of Dispose(bool).
  3. Keep your LoggingClass objects around longer. Yes, what you have demonstrated is a properly implemented using statement, but you'll end up creating & destroying thousands of objects in your code. Create one LoggingClass object, and keep it around for the entire length of the program, not just for one log statement.
  4. Call LoggingClass.Dispose() when your application is about to exit. Do this manually (not with a using statement).
2
  • Just a note here: You do not need the ~LoggingClass() override if the object does not own unmanaged resources. Commented Apr 5, 2012 at 23:59
  • @DavidYaw that is my entire class, so i guess i'm missing something completely, using what I have above how would i modify the class?
    – user222427
    Commented Apr 6, 2012 at 1:37
0

No, this isn't the proper use of IDisposable, namely because you don't appear to actually be disposing anything. This is also the cause of your error; you've taken out some automatically generated code. Put it back (it looks something like this) and follow the comments:

private bool disposed = false;

protected virtual void Dispose(bool disposing)
{
    if(!disposed) {
        disposed = true;

        if(disposing) {
            // Clean up managed resources, like files or GDI objects
        }

        // Clean up unmanaged resources, like COM components
    }
}
-1

why am I getting the error No Overload for method 'dispose' takes 1 argument.

Because you don't have an overload accepting a bool for the Dispose() method.

is this the proper use of Dispose() and also

It's hard to tell. Your class has no managed or unmanaged resources that you have shown, so the method essentially does nothing. (Actually it calls GC.SuppressFinalize(this) which will just make things worse)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.