Module vrs Procedure level variable declarations 3

[Aidy680]

I wonder if someone might be able to clarify a difference of opinion we have:

I have 3 sub procedures which use a recordset of the same name. For efficiency sakes, I believe it's better to declare this variable at module level.

My colleague believes it should be declared 3 seperate times within each seperate Sub. I disagree.

Can anyone 'assist'?!

BTW please dont comment on whether or not I should be using a recordset variable with the same name in each procedure. That's a different argument!

 

[MajP]

Good programming always tries to limit both the scope and lifetime of a variable

http://visualbasic-programming.blogspot.com/2007/11/scope-and-lifetime-of-variables.html

• The scope (or visibility) of a variable is the portion of code from which that variable can be accessed. For example, a variable declared with the Public attribute in a BAS module is visible—and can therefore be read from and written to—from anywhere in the application, whereas if the variable is declared Private, it's visible only from within that BAS module.
• The lifetime of a variable is the time period during which that variable stays alive and uses memory. The lifetime of the Public variable described in the previous paragraph coincides with the application's life, but in general this isn't always the case. For instance, a local dynamic variable in a procedure is created each time Visual Basic executes that procedure and is destroyed when the procedure exits

At the same time you do not want to open and close recordsets. So if you call only one procedure at a time then local is the correct answer. But if you always call three procedures. Then neither of you are correct. Here is the proper solution if you always use all three procedures in a row.

public sub executeProcedures()
dim rs as dao.recordset
set rs = ....
call procedure1(rs)
call procedure2(rs)
call procedure3(rs)
end sub

The scope is local to executeProcedures, and the lifetime ends at the end of execution, and the rs is opened only once.

 

[Aidy680]

Thanks MajP. Not looking good for me!!

Does anyone have a different view (and can back it up with a link to somewhere authorative as per the above!)

 

[MasterRacker]

You mention using recordsets with the same name. Just to be clear: are you using the same set of data in each procedure or are you trying to reuse the recordset to query different data in each procedure?

There's nothing wrong with local variables in different procedures having the same name - they're local to their procedures and don't affect each other.

If you mean you are loading a recordset with certain data and using that same data in different places than MajP's solution is correct. You wouldn't want to requery the same data over and over just to stick with local variables.

Jeff
[small][purple]It's never too early to begin preparing for [/purple]International Talk Like a Pirate Day
"The software I buy sucks, The software I write sucks. It's time to give up and have a beer..." - Me[/small]

 

[Aidy680]

Jeff the recordsets aren't parsed from one procedure to another; their scope is limited to the procedure in which they're used and then they're destroyed.

The recordsets, within the different subs, hold different data, at different times. The data is sourced from the same tables though.

I understand that in days gone by it was bad form to declare a variable once, at module level, due to memory constraints. However nowadays that same concern doesn't really apply.

 

[Golom]

The recordsets, within the different subs, hold different data, at different times.

Given that, its almost certain that you want to create different ones in the different subs. The reason being that there is a risk that, if declared only once at module level, you will be getting data loaded somewhere else.

Declaring a recordset as in

Dim myRS as ADO.Recordset

doesn't go anywhere near the database. It just populates the program's symbol table and, before you load some data into it, it occupies a trivial amount of memory.

Sorry but the view around here seems to take a dim (no pun intended) view of your module-level declarations.

 

[Aidy680]

Cheers all for your feedback.

Think I'll have to concede this one.

 

[MajP]

Will this work for you? Sure. Is it good coding practice? No.

Does anyone have a different view (and can back it up with a link to somewhere authorative as per the above!)
[/code]
Sorry, there is no different view that is correct. I provided the correct answer. Sloppy code will work, but good programmers always limit the scope and lifetime of variables.

I understand that in days gone by it was bad form to declare a variable once, at module level, due to memory constraints.  However nowadays that same concern doesn't really apply

It really has nothing to do with memory, it has to do with protecting your variables. At the module level it is floating out there begging for something to mistakenly use it. Think of it like a rifle. When you want to use it you take it out, limit who has access to it, and lock it back up as soon as you are done. Of course you could leave it in the back seat of your car (module level) so it is always ready to use. As you write object oriented programs you will shield the user from you the inner wokings of your classes and only expose certain methods and properties.

 

[alvechurchdata]

Does anyone have a different view

My only "different" view is that sometimes code that works is all that's required. If I were writing a quick and dirty one-shot utility then I might well use a module-level variable rather than passing rs back and forth as a parameter.

Otherwise I'm with MajP, variables floating around in the back of the car are just waiting to blow your foot off.

I might even have gone one step further in his example and added:

rs.Close
Set rs = Nothing

at the end of the sub just to make it explicitly plain that rs is no longer in use.

Geoff Franklin

http://www.alvechurchdata.co.uk

 

[Aidy680]

MajP thanks for your contributions but your tone can be somewhat stuffy. Not really necessary in these forums.

Thanks again to all of the other contributors.

 

[lameid]

MajP, I found your post concise and accurate...
Have a star.

All, I agree with alvechurchdata, you ought to close and empty the variable too, in order to be complete.

 

[SkipVought]

Au contrair, Aidy680!

You got sound, professional and best practices advise. I'd stuff it, if I were you. But then again, I'm not.

Skip,

_{Just traded in my old subtlety...
for a NUANCE!}

 

[Aidy680]

Whoa look guys no-one is suggesting that MajP's advice wasn't professional, helpful and welcome.

Just that maybe you experts need to lighten up a tad every now and again.

Can I break for the weekend now?!