1

Let's say I have a class that has a MyCustomDatabaseAccess as a data member. MyCustomDatabaseAccess has a Dispose() method. MyCustomDatabaseAccess is the middleware class that accesses the database.

public class MyClass {
   private MyCustomDatabaseAccess db_access;
}

Does MyClass need to implement IDisposable interface?

My solution right now is to have do something like this:

    public class MyClass {
       private MyCustomDatabaseAccess db_access;

       public void GetDBResults () {
          db_access = new MyCustomDatabaseAccess();
          DataTable dt = db_access.ExecuteStoredProc(param1, param2, etc..);

          //do stuff with results

          db_access.Dispose();

    }

}

From what I read on MSDN, another way to make sure that this object is disposed of properly would be to have MyClass implement IDisposable interface, then implement a Dispose() function, then call it in the class that calls an object of MyClass. see this for more info http://www.devx.com/dotnet/Article/33167/0/page/3

Which way is preferable and why? thanks!

sarsnake
  • 26,667
  • 58
  • 180
  • 286
  • thanks for your comments, they are very relevant to the question asked. Appreciate it, John Hartsock. – sarsnake Dec 10 '10 at 18:16
  • 3
    It is relevant because it effects how willing people will be to help you. – Ed S. Dec 10 '10 at 18:16
  • 5
    @gnomixa: I have to second John Hartsock. I was going to post an answer, but your snarky comments (and obvious lack of willingness to be a responsible user here) have changed my mind. – Adam Robinson Dec 10 '10 at 18:18
  • @gnomixa...This is a great site and tool to all developers. Using this site as it was designed not only assists you but other developers. This also keeps others coming back for more. Quotes from the FAQs "As you see new answers to your question, vote up the helpful ones by clicking the upward pointing arrow to the left of the answer." ..."When you have decided which answer is the most helpful to you, mark it as the accepted answer by clicking on the check box outline to the left of the answer....Doing this is helpful because it shows other people that you're getting value from the community." – John Hartsock Dec 10 '10 at 18:23
  • Why is `db_access` a member and not a local variable of GetDBResults? – CodesInChaos Dec 10 '10 at 18:25
  • 1
    John, I understand. Many of my questions are about quite obscure topics such as particular Jquery library that only 2 people here use or about Compact framework. Have you ever thought that perhaps, NONE of the answers were helpful? When I remember, I do answer my own question and mark it as an asnwer 24 hours later, but SO is not my full time job so please forgive me if I don't do it all the time. – sarsnake Dec 10 '10 at 18:27
  • I haven't used any Jquery grids yet - it was a question I was asking in advance for a collegue. As far as a serial port question - I ended up doing it differently and forgot to post my way of doing it. As I said, my priorities are not on SO, my job is my priority. My job is the reason why I am on SO. Sometimes, I forget to mark the answer. Several times the project has been canceled or I ended up using different tools altogether. My point is, again, if you don't feel like answering m y question because of whatever reason (I am mooch/you woke up on the wrong side/you haven't had coffee), DON'T. – sarsnake Dec 10 '10 at 19:11
  • and this http://meta.stackexchange.com/questions/23321/is-it-appropriate-to-comment-on-peoples-accept-rate – sarsnake Dec 11 '10 at 00:14

4 Answers4

2

I would suggest using one of the following two approaches:

  1. Wrap the IDisposable object in a using block.
  2. Keep a reference to the IDisposable object inside your class and have your class implement IDisposable. Dispose the object in your class's Dispose method.

Which you should choose depends on the required lifetime of the object.

  • If you only need it for the duration of a single method call then the first option - a using block - is preferable because it is simpler and harder to get wrong. The using block (almost) guarantees that Dispose will be called for you as soon as the object is no longer needed - even in the case where an exception is thrown.
  • If the disposable object must last longer than the duration of a single method call then you cannot use the first option, so you should use the second - i.e. your class should implement IDisposable.
Mark Byers
  • 811,555
  • 193
  • 1,581
  • 1,452
  • You probably don't want to create and destroy database access objects every time you run a query (i.e., #1) – Ed S. Dec 10 '10 at 18:15
  • Thanks Mark, wouldn't using essentially do what I am doing right now? Open it, use it and kill it? thanks. – sarsnake Dec 10 '10 at 18:17
  • 1
    @Ed: That depends on what they are. For example, a connection-pooled `SqlConnection` is very lightweight and, actually, *is* designed to have a very short lifespan. – Adam Robinson Dec 10 '10 at 18:19
  • 3
    @gnomixa, what happens if there is an exception after the object is created and before it is disposed. Are you sure it will still be disposed in this case? Using a using statement ensures that. – Sam Holder Dec 10 '10 at 18:20
  • oh right, thanks Sam! I forgot about that little neat feature. – sarsnake Dec 10 '10 at 18:30
  • Personally, 'using' is rather horrible sugar- especially as it often causes problems with potential null references to objects within the block that need to be promoted to being declared outside. However, it can simplfy avoiding having to implement IDisposable in situations like this. IDisposable was not thought-through very well. – nicodemus13 Jan 06 '11 at 09:14
1

If you really are maintaining a connection to a database (and its not just a sample code you wanted to post here), you are doing the right thing right now. That is opening database connection, querying for data and closing/disposing it.

On the other hand, IDisposable.Dispose() method should be used if your resources take up a lot of memory but are not time critical to be closed/disposed as soon as possible. When your object is no longer referenced and GC needs memory, your object will be collected and Dispose() method will be closed on it which will dispose your unmanaged large object.

Sam Holder
  • 32,535
  • 13
  • 101
  • 181
decyclone
  • 30,394
  • 6
  • 63
  • 80
  • I agree completely but would like to add that if the OP surrounds the code with a try/catch/finally they can ensure that the dispose is called even if an exception is thrown. – SRM Dec 10 '10 at 18:18
  • @SRM: The `using` block does that in a much less verbose manner. – Adam Robinson Dec 10 '10 at 18:20
  • 1
    `Dispose()` will not be called automatically on collection. You need to implement some form of finalizer, preferably in the form of a SafeHandle. – CodesInChaos Dec 10 '10 at 18:35
1

I think you just need to ensure your objects are disposed.

if you are doing this in your method, even if there is an unexpected error, then I think you are ok. If you hold on to unmanaged resources in your class then implementing IDisposable is a way to ensure that you get a chance to dispose resources when your object is finalized, or to give your users a way to dispose of the resources explicitly.

If you are only creating and using the resources in a method and not holding references in the class then as long as you are ensuring they are disposed in the method (either by doing it manually , or more easily, by wrapping then in a using block), then you should be ok I think.

Sam Holder
  • 32,535
  • 13
  • 101
  • 181
  • 4
    "IDisposable is a way to ensure that your resources are disposed when your object is finalized" This is misleading or wrong. To release resources during finalization you use something like a SafeHandle. IDisposable is only used for explicit/deterministic release of unmanaged resources and typically just forwards the Dispose to the SafeHandle. A high level object like the one the OP is implementing should almost never have a finalizer. – CodesInChaos Dec 10 '10 at 18:24
  • @CodeInChaos is completely correct. Contrary to the implication in the answer, **IDisposable does absolutely nothing special**, other than allowing the use of language conveniences like `using`. `IDisposable` does *nothing* in finalization automatically (and, in fact, should not; only classes that *actually* have unmanaged resources should account for them in finalization). – Adam Robinson Dec 10 '10 at 18:55
  • Apologies if my answer was misleading, I only meant that implementing IDisposable is a way to ensure that you are given an opportunity to dispose of your resources. I was editing that exact line (as I realised on re reading it that it was misleading) when @CodeInChaos was making his comment, the timings would suggest. I did not mean to imply that by implementing IDisposable you somehow got automatic disposal of your resources. As has been pointed out you still need to manage this disposal yourself. – Sam Holder Dec 10 '10 at 19:22
0

Yes, implementing IDisposable is how you tell clients of your code "hey, you need to call dispose on this object because it maintains references to unmanaged resources."

Ed S.
  • 122,712
  • 22
  • 185
  • 265