Closing Database Objects: Explicitly Required?

[mdlueck]

One disposes and sets the values to nothing. The other does not. The one that does not leaks memory.

Sounds like an excellent sample, DrallocD!

Affirms the care I endeavor to put into my software development to clean up objects at the same scope as objects are defined at. I have never noticed memory leak type behavior out of Access. "Computing That Just Plain Works!"

 

[speakers_86]

Well, my mistake then. Ill take a look again.

On a different note, is that 22 seconds? That seems really slow for so few records. I can get over 600 in less time.

 

[ChrisO]

speakers_86

Your test database is attached here.

Because I run XP I had to change the DAO reference but apart from that the only thing added is the means to time the FillList subroutine.

I don’t know if it is slow or not but it is consistent.

Regards,
Chris.

 

[ChrisO]

Mdlueck

I know I’m nit-picking but we use computers and we need to be.

You say:-
>>Sounds like an excellent sample, DrallocD!<<
But that statement does not indicate that you actually tried DrallocD’s sample.

So I need to ask; did you try it and did it behave the way DrallocD says it does?
----------



DrallocD

If it is your intention to prove me wrong by posting a sample, why post in accdb file format when my signature clearly indicates I use Access 2003?

It is this sort of attention to detail, or lack of it, which is important.
----------


Chris.

 

[DrallocD]

Chris,

I have attached a 2003 version for you to test.

 

[ChrisO]

DrallocD

Thanks for the Access 2003 demo, but your testing is faulty.

This is your code:-

Private Sub cmdWith_Click()
    Dim c1 As New Class1
    Dim c2 As New Class2
    c1.FillString
    c2.FillString
    c1.remember1 c1
    c1.remember2 c2
    c2.remember1 c1
    c2.remember2 c2
    
    Pause
    
    c1.Dispose
    c2.Dispose
    Set c1 = Nothing
    Set c2 = Nothing
End Sub

Private Sub cmdWithout_Click()
    Dim c1 As New Class1
    Dim c2 As New Class2
    c1.FillString
    c2.FillString
    c1.remember1 c1
    c1.remember2 c2
    c2.remember1 c1
    c2.remember2 c2
    Pause
   
End Sub

This diccusion is about the need to set objects to Nothing.
Therefore a valid test is to have one example where they are set to Nothing and one where they are not set to Nothing.
Your example does two things.
In one test you Dispose and set to Nothing.
In the other test you do neither of the two.

The valid test should have been to only test the setting to Nothing.

Hence the test code should have been this:-

Private Sub cmdWith_Click()
    Dim c1 As New Class1
    Dim c2 As New Class2
    c1.FillString
    c2.FillString
    c1.remember1 c1
    c1.remember2 c2
    c2.remember1 c1
    c2.remember2 c2
    
    Pause
    
    c1.Dispose
    c2.Dispose
    Set c1 = Nothing
    Set c2 = Nothing
End Sub

Private Sub cmdWithout_Click()
    Dim c1 As New Class1
    Dim c2 As New Class2
    c1.FillString
    c2.FillString
    c1.remember1 c1
    c1.remember2 c2
    c2.remember1 c1
    c2.remember2 c2
    Pause
   
    c1.Dispose
    c2.Dispose
   
End Sub

When running tests, it is a common mistake to change too many things at once.
When we test only removing the setting to Nothing, both methods behave exactly the same.

Hence the conclusion, from your own test, is that the setting to Nothing is not required.

Now, if people can not test their own code properly, and they advise other people, then they are not part of the solution they are part of the problem.

Do you have any more samples you would like to put forward?
Is there any more advice on this matter you would like to give?

Chris.

 

[DrallocD]

What is it the dispose does again?

Public Sub Dispose()
    Set remember1backer = Nothing
    Set remember2backer = Nothing
End Sub

As you can plainly see from the few lines of code, all the Dispose() method does is sets the values to Nothing at the level that they were set in the class. You chose to ignore this to stop yourself having to admit the truth. It proves that the GC can leak memory which you don't even acknowledge to be true even though the memory usage was constantly increasing when you were running the test.

This is what should be done in a Dispose() or Close() event of any class. The whole argument is about the fact that the object knows how to close itself and release any resources, not the GC.

Not closing or disposing and letting an object go out of scope is a lazy approach which fortunately for you Microsoft coders have plugged for the main part so that you can continue to code as you do. This is not the case in all objects so best practice says you should Close and set to Nothing.

This example clearly shows how the GC leaks memory if you do not set values to Nothing at the appropriate level.

You have far too much invested in not accepting good programming practice that nothing I say will ever change your mind. If it did, imagine how much code you would have to go back and write correctly!

You may want to reconsider your earlier remark:
>>Do not attempt to close anything which is about to go out of scope.<<

 

[ChrisO]

For your information…

----------
Firstly:-
Option Explicit was not turned on behind the Form.
That does not stand in good stead of an experienced programmer.
----------


----------
Secondly:-
Dim c1 As New Class1
is not the same as:-
Dim c1 As Class1
Set c1 = New Class1

Unless the Class is called otherwise, simply using:-
Dim c1 As New Class1
will not call the Private Sub Class_Initialize().
If the Private Sub Class_Initialize() is not called then neither will Private Sub Class_Terminate().

If :-
Dim c1 As Class1
Set c1 = New Class1
is used, both the Private Sub Class_Initialize() and Private Sub Class_Terminate() are called, even if no other call to the Class is made, except as below.
----------


----------
Thirdly:-
A Class which reference itself will not simply go out of scope.
In fact Private Sub Class_Terminate() will not be called even if you use:-
Dim c1 As Class1
Set c1 = New Class1

If Private Sub Class_Terminate() was called, and it tried to destroy the reference to itself, then the destruction of that self reference could call Private Sub Class_Terminate(). If that happened you could easily end up in a loop which is trying to destroy itself.
----------


----------
Fourthly:-
The real problem with your code is that you created a self reference within the Class module. That self reference needs to be destroyed separately without the possibility of creating a circular reference due to its destruction. The c1.Dispose you had did exactly that but you removed it from the test you intended to show memory leakage. Was that because you wanted to demonstrate a failure or was it because you programmed it incorrectly? Maybe it was both, but please don’t expect to get away with it.
----------


----------
Fifthly:-
In all of the above cases, setting to Nothing does exactly what it is supposed to do…
Absolutely NOTHING, except demonstrate the programmer’s ineptitude.
----------

And that is the main reason the programmer needs to be able to prove they have a reason for doing something.

You can not expect everyone else to simply fall down in awe of big words and fancy concepts.
You need to be able to prove that what you are doing is correct if only for your own self esteem.


Chris.

 

[ChrisO]

DrallocD

>> You have far too much invested in not accepting good programming practice that nothing I say will ever change your mind. If it did, imagine how much code you would have to go back and write correctly!<<

Rhetoric…
>>You have far too much invested<<
I think so, but obviously not enough.

>>not accepting good programming practice<<
It is only an assumption that it is good.

>>nothing I say will ever change your mind<<
Correct. I don’t look at words I look at provable facts.

>>imagine how much code you would have to go back and write correctly!<<
That is possible, but I do test code better than you do and you proved it.

----------

Now that you know what was wrong with your first demo, if you didn’t before you uploaded it, would you care to upload another one? (Access 2003 please.)

Chris.

 

[DrallocD]

In response:

Firstly
>>Option Explicit was not turned on behind the Form<<
>>That does not stand in good stead of an experienced programmer.<<
Actually the opposite it true - it is always best to not assume that this is turned on at the database level in the envioment that I work.

Secondly:
>>Dim c1 As New Class1<<
>>is not the same as:-<<
>>Dim c1 As Class1<<
>>Set c1 = New Class1<<

>>If the Private Sub Class_Initialize() is not called then neither will Private Sub Class_Terminate().<<

Totally wrong. These are identical but irrelevant as the sample does not contain this event. Add it and you will see that it is called (as will everyone else who will then confirm my first statement).

Thirdly:
>>If Private Sub Class_Terminate() was called, and it tried to destroy the reference to itself, then the destruction of that self reference could call Private Sub Class_Terminate(). If that happened you could easily end up in a loopwhich is trying to destroy itself.<<

And if my aunt had balls she would be my uncle! Seriously, do you think I fell out of high school yesterday? This is total BS.

Fourthly:
>>A Class which reference itself will not simply go out of scope.<<
Again, wrong. It is the fact that Class1 references Class2 and Class2 references Class1 that prevents it going out of scope. This is a common problem which is why you need to set both to Nothing for the GC to be smart enough to clean them up. I had hoped that you would have at least picked up how I caused the memory leak!

Fifthly:-
In all of the above cases, setting to Nothing does exactly what it is supposed to do…
Absolutely NOTHING, except proving my point and your ineptitude.

I now feel sorry for your users and sooo glad that you do not work for me.

 

[ChrisO]

>>I actually feel bad for you, you are now having to lie to try to defend your point.<<

DrallocD, that is really not a good idea.

Chris.

 

[speakers_86]

Hmm...this used to be such a nice conversation.

 

[DrallocD]

I find one things slightly strange. ChrisO is the only person on this site that I have thanked to date for useful advice.

Developers are typically more interested in helping each other than trying to convert them to their own style of coding so this may well be the only issue that we disagree on. I hope that we can overcome it and help each other in the future.

 

[DrallocD]

@Speakers_86
From your previous post about timing - did you realize that all recursive programming can be converted to non-recursive? I have not seen your code but please feel free to ask any questions about improving performance either publically or privately if performance is a problem for you.

 

[ChrisO]

DrallocD

Firstly
>>Option Explicit was not turned on behind the Form<<
>>That does not stand in good stead of an experienced programmer.<<
Actually the opposite it true - it is always best to not assume that this is turned on at the database level in the envioment that I work.

I can not comment on the environment in which you work but maybe you would like to explain why it is turned off behind the Form and on in both Class modules.

If you have a valid reason for that, apart from forgetting to turn it on behind the Form, it could be useful for others to know.

----------

Secondly:
>>Dim c1 As New Class1<<
>>is not the same as:-<<
>>Dim c1 As Class1<<
>>Set c1 = New Class1<<

>>If the Private Sub Class_Initialize() is not called then neither will Private Sub Class_Terminate().<<

Totally wrong. These are identical but irrelevant as the sample does not contain this event. Add it and you will see that it is called (as will everyone else who will then confirm my first statement).

It’s not totally wrong at all. I tested what I said before I said it. If you wish to ask one specific question at a time I will answer it.

Also note that I did not say the sample you posted had Private Sub Class_Initialize() and/or Private Sub Class_Terminate() events. What would be the point of me saying it had those events when others can download your sample and look for themselves?

Again, if you want to know why then ask, don’t assume

----------

Thirdly:
Not worth commenting on.

----------

Fourthly:
>>A Class which reference itself will not simply go out of scope.<<
Again, wrong. It is the fact that Class1 references Class2 and Class2 references Class1 that prevents it going out of scope. This is a common problem which is why you need to set both to Nothing for the GC to be smart enough to clean them up. I had hoped that you would have at least picked up how I caused the memory leak!

Incorrect. You do not need two Class modules to cause that. You only need one selfreferencing Class module for that to happen. Again, I tested it before posting.

----------

Fifthly:-
In all of the above cases, setting to Nothing does exactly what it is supposed to do…
Absolutely NOTHING, except proving my point and your ineptitude.

I don’t know who was supposed to have said that but it wasn’t me.

----------



If you want some explanation of anything I have said then just ask but do not make assumptions.

Note 1. This thread is about the failures that supposedly occur when DAO objects are not closed and/or set to Nothing.

Note 2. Members of this site should not be told to grow up, nor should they be called liars. The reason I did not ban you was because those words were directed at me and I felt it might be seen as a banning due to self interest. Don’t bet on that happening again.


Chris.

 

[DrallocD]

Okay, we can take each one individually if you like.

Point 1:
>>Option Explicit was not turned on behind the Form<<
>>That does not stand in good stead of an experienced programmer.<<

I consider this irrelevant because it is not needed and has no impact on this test since I do declare all of my variables before I use them.

We can use MSDN as a reference point if you like:

http://msdn.microsoft.com/en-us/library/y9341s4f(v=vs.80).aspx

I quote:

When Option Explicit appears in a file, you must explicitly declare all variables using the Dim or ReDim statements. If you attempt to use an undeclared variable name, an error occurs at compile time.

If this was not sample code I would have used better variable names, maybe added comments and set Option Explicit On.

Can we agree on that this is not relevant to our discussion and move on to point 2?

 

[speakers_86]

I believe ChrisO's point is simply that it a good habit to have Option Explicit on, and that leaving it off is a sign of an inexperienced programmer.

 

[ChrisO]

DrallocD

>>Can we agree on that this is not relevant to our discussion and move on to point 2?<<
No we can’t because it is entirely relevant to this thread.

----------
In post #16 you talk about immutable strings and to support your assertion you quote Microsoft:-
http://msdn.microsoft.com/en-us/library/ms234766(v=vs.80).aspx
In that link to Microsoft, Microsoft uses this line of code:-
Dim myString As String = "This string is immutable"
That line of code will not even compile in Access.
----------

----------
Now when it comes to Option Explicit you quote Microsoft with:-
http://msdn.microsoft.com/en-us/library/y9341s4f(v=vs.80).aspx
To quote Microsoft from that link:-
“Optional. Enables Option Explicit checking. If On or Off is not specified, the default is On.”

But in Access it is Off by default. In fact Luke Chung from FMS:-
http://www.fmsinc.com/toplevel/about.htm
has been trying to get Microsoft to turn it On by default for many years.

And that is a fact because it was a public discussion I had with him when he came to Australia a couple of years ago. And another member from this site was present at the time.
----------


So what’s going on with Microsoft; have they made two errors in two links?

And why did you delete this from your reply in post #36 and leave no reason for the edit:-
*On*
Optional. Enables *Option Explicit* checking. If *On* or *Off* is not specified, the default is *On*.
*Off*
Optional. Disables *Option Explicit* checking.


The fact is, DrallocD, that both of those links you supplied to support your assertions are links to Visual Basic (VB) not Visual Basic for Applications (VBA).

That is an error which any experienced (VBA) programmer should not make.

So I think it is entirely relevant to this thread to find out why you are trying to support your position by posting links to the wrong Microsoft product. I think it is also important to find out why you thought it necessary to remove the above bolded section from your reply in post #36 with no reason for the edit.

Chris.

 

[DrallocD]

Hi Chris,

I am trying to help you by proving the need to close database objects but you keep going off on tangents and questioning irrelevant issues like option explicit. You are correct that in MS Access option explicit is off by default but my point is that sample code and production code are not the same. I also had badly named variables and no error handling unlike my production code. Adding option explicit makes no difference to the execution of the code or the result.

I did not jump on the immutability of strings earlier in the thread even though you are wrong. Strings are immutable in all versions of VB. If you would like to start a new thread I can prove that to you. I can show how using a mutable type can be 1000's of times faster than an immutable one in VBA and bring processing times down from hours to seconds.

I am happy to invest my time proving that you need to close database objects but you in turn must agree to keep on topic and keep the thread positive. I don't mind agreeing to this if you are willing since I accept 50% of the blame.

can we agree that this is about (and only about):

Set x = Nothing preventing memory leaks and rs.Close causing unexpected errors?

It is not about: nit-picking / disrespecting / point scoring.

I am currently investigating a reproducable error with rs.Close but I will only proceed if we agree to the new terms.

The code currently looks like this but I would like to investigate further and create a stand-alone test:

    For i = 1 To 800
        Set rs = db.OpenRecordset("SELECT * FROM table1 where ID = " & i, dbOpenDynaset, dbDenyWrite)
        rs.Edit
        rs!txt = "This is " & i
        rs.Update
        
        'rs.Close
    Next i

This always fails where i = 2, if I uncomment rs.Close it always works correctly - and yes, I know I could have done this with a single update statement and i is not declared.

 

[ChrisO]

DrallocD

>>Adding option explicit makes no difference to the execution of the code or the result.<<

Everybody should know that but it is not my point.

My point is that in order to try and prove your assertions about immutable strings and the default state of Option Explicit you quoted the wrong Microsoft product.

Those two mistakes, and the deletion of information from post #36 without a stated reason, cast serious doubt on your replies.

Can you explain those errors?

Until such time as I can have confidence in your replies there is little point in moving on.

Chris.