Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

Today, I came across a code example like the following:

Option Explicit

Sub MySub()
    Dim iCount As Integer

    For iCount = 0 To 100
        Debug.Print "Arbitrary Code Here"
        If iCount Mod 2 = 0 Then

            Dim myString As String 'This is the line I'm curious about

            myString = iCount
            Debug.Print myString
        End If
    Next iCount
End Sub

Once a variable goes in scope in VB6 and/or VBA, it stays in scope throughout the procedure.

So, is it a bad programming practice to declare a variable in a loop? Is it better to declare myString either at the top of the procedure or, at the very least, above the FOR LOOP?

UPDATE: There are some good answers here. I hoped my question was objective, but there's enough personal opinion here to, IMHO, make it subjective.

I'm marking Jeff's answer as the "right" one; mainly because I agree with what he says. If I were to answer my own question, it would be 'yes' and for the same reason as Jeff's.

share|improve this question

3 Answers

up vote 5 down vote accepted

There are a few things to consider here. First we have to consider if the position will matter to the compiler, which depends on the language. VB6 will scope that variable to the function (or sub in this case), so it doesn't matter too much there. In VB.NET, the variable will be scoped to the containing block (in this case the loop), so declaring inside of the loop would let us reuse that same variable name in some other block.

In terms of maintainability, there are two good reasons to put the declaration as close as possible to the first use of the variable. One, it improves readability. Two, it simplifies future refactoring -- imagine that you wanted to move that loop to its own function, that's easier if the variable is right there with the loop.

short answer: In VB6 I think it's best to declare just before the loop. This is mostly so that nobody thinks that the variable will be re-initialized every time the loop runs.

share|improve this answer
up vote 3 down vote

In most languages, where the variable scope is defined by the block it's defined (such as VB), it's preferred to declare the variable close to where it's going to be used. In others with function-level scope (such as javascript, where a variable declared anywhere is scoped to the whole function where it was defined), I've usually seen it being declared at the beginning of the function.

share|improve this answer
up vote 2 down vote

It is bad programming practice to declare a variable in a loop, when it's only going to be re-used in the loop, even if in VB it won't be re-initialized every time.

A good link on declarations in VB is here.

One quote:

The recommended practice is to declare all local variables within a procedure immediately following the Sub or Function header and prior to any executable statements (although VB will allow you to declare variables anywhere within the procedure).

In a long enough module, you'd want a variable with limited scope to be DIMmed close to where it's being used, but always outside of a loop.

share|improve this answer
Why should it be outside of the loop? You've just said it's preferable to declare it close to where it's used, and if it's only used in the loop that's the closest you can get. Why the double standard? – Mark Peters May 20 '11 at 13:13
@Mark, it's not a double standard. You get it as close to the loop as possible but not in the loop. So in this small case, it should be right with the other DIM statement. – Lance Roberts May 20 '11 at 15:19
The question is about VB6, which treats all variables declared in the procedure as procedure-scoped, and initializes them once. It's a surprising thing the first time you run into it, and leads to defensive measures like manually writing an explicit initializer at the top of every loop. – Jeff Paulsen May 20 '11 at 16:16
2  
to clarify, I agree with Lance, but not for the reason of multiple re-initialization. I agree because putting the declaration in the loop makes the reader THINK there will be reinitialization. – Jeff Paulsen May 20 '11 at 16:24
@Jeff, thanks, I've edited the answer. Learn something new every day on Stack Exchange. – Lance Roberts May 20 '11 at 16:35

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.