Is this a bug I see before me, or an incomplete understanding of scope?

T

teddysnips

Is this a bug?

I open a DataReader dr from a SQL Server SPROC

Do While dr.Read
Dim ClubDay As New ChildrensClubDay
With ClubDay

Dim MPInfo As PaymentInformation
If Not dr.GetDateTime(1) = Date.MinValue Then
MPInfo = New PaymentInformation(dr.GetDateTime(1), New
IndividualKey(dr.GetString(2)))
End If
.MorningPaymentInfo = MPInfo
' Get afternoon paymentinfo
Dim APInfo As PaymentInformation = Nothing
If Not dr.GetDateTime(3) = Date.MinValue Then
APInfo = New PaymentInformation(dr.GetDateTime(3), New
IndividualKey(dr.GetString(4)))
End If
.AfternoonPaymentInfo = APInfo

End With
ChildrensClubDayList.Add(ClubDay)
Loop

I've removed a lot of the lines for conciseness. (The
PaymentInformation class shown below)

The datareader contains information about a ChildrensClubDay, with a
separate row for each day.

The way this bug expressed itself (if it is a bug) was thus. For a
particular set of data the first two rows contained no booking
information, and so

dr.GetDateTime(1) returned nothing, which did not satisfy the If
condition. Similarly, dr.GetDateTime(3) returned nothing.

On the third day, there WAS a booking, and so the conditions were met,
and the lines

MPInfo = New PaymentInformation(dr.GetDateTime(1), New
IndividualKey(dr.GetString(2)))
APInfo = New PaymentInformation(dr.GetDateTime(3), New
IndividualKey(dr.GetString(4)))

were executed leading to the lines

..MorningPaymentInfo = MPInfo
..AfternoonPaymentInfo = APInfo

being correctly populated.

HOWEVER. When the next two days were processed, the conditions were
NOT met, BUT the state of MPInfo and APInfo had persisted from the
previous iteration of the loop, thus effectively bypassing Dim MPInfo
As PaymentInformation etc.

I solved the problem by writing Dim MPInfo As PaymentInformation =
Nothing.

But surely I shouldn't have had to. Don't variables scoped in loops
get collected and destroyed on each iteration of the loop?

Jess Askin


Public Class PaymentInformation
Public PurchaseDate As Date
Public PurchasedBy As IndividualKey

Sub New()

End Sub

Sub New(ByVal purchaseDate As Date, ByVal purchaseBy As
IndividualKey)
Me.PurchaseDate = purchaseDate
Me.PurchasedBy = purchaseBy
End Sub

#Region "Helper"

Public Shared Sub GetValues(ByVal pi As PaymentInformation, _
ByRef purchaseDate As Date, ByRef
purchasedBy As String)
' Get payment info with null validation
If Not pi Is Nothing Then
purchaseDate = pi.PurchaseDate
If Not pi.PurchasedBy Is Nothing Then purchasedBy =
pi.PurchasedBy.Key
End If
End Sub

#End Region

End Class
 
T

Tim Patrick

The lifetime of a block variable is still the entire procedure, not just
the block. It is only created once, when the procedure is entered, not when
the block is entered. The scope of the block variable is the block; you can't
access it outside of the block. The following code shows how the value of
x lasts the entire lifetime of the procedure. x will take on the values 1
through 6 in each message box.

For outer As Integer = 1 To 2
For counter As Integer = 1 To 3
Dim x As Integer
x += 1
MsgBox(x)
Next counter
Next outer

This fact of a block variable having a lifetime for the whole procedure is
documented in the Visual Studio documentation. Any assignment you attach
to the Dim statement is processed each time it is encountered.
 
M

Marina Levit [MVP]

I don't know if it's a bug or not. My guess is the compiler is optimizing
this and moving the variable declaration outside the loop.

However, I would say the real problem is that you are declaring variables
inside a loop. This makes code extremely hard to read and figure out what is
going on. There are if statements, then all of a sudden variable
declarations, etc. Someone reading this code (like me) is likely to going
to have a hard time figuring out what it was your code was trying to
accomplish here and under what circumstances.

You are much better off declaring all your variables outside the loop and
making it as straight forward as possible to see what the purpose of the
loop is.
 
T

teddysnips

Marina said:
I don't know if it's a bug or not. My guess is the compiler is optimizing
this and moving the variable declaration outside the loop.

However, I would say the real problem is that you are declaring variables
inside a loop. This makes code extremely hard to read and figure out what is
going on. There are if statements, then all of a sudden variable
declarations, etc. Someone reading this code (like me) is likely to going
to have a hard time figuring out what it was your code was trying to
accomplish here and under what circumstances.

You are much better off declaring all your variables outside the loop and
making it as straight forward as possible to see what the purpose of the
loop is.

Absoloopy. With honour and pride I can put up my hands and say "NOT MY
CODE!" I was just engaged to maintain it.

Edward
 
B

Branco Medeiros

Is this a bug?
Do While dr.Read
Dim ClubDay As New ChildrensClubDay
With ClubDay

Dim MPInfo As PaymentInformation
When the next two days were processed, the conditions were
NOT met, BUT the state of MPInfo and APInfo had persisted from the
previous iteration of the loop, thus effectively bypassing Dim MPInfo
As PaymentInformation etc.

I solved the problem by writing Dim MPInfo As PaymentInformation =
Nothing.

But surely I shouldn't have had to. Don't variables scoped in loops
get collected and destroyed on each iteration of the loop?
<snip>

Nope. This is allways a source of misunderstanding, even though it's
well documented.
If you use block variables, initialize them (unless you want such side
effects, as preserved values between cycles).

See:
http://msdn2.microsoft.com/en-us/library/1t0wsc67.aspx

For a lengthier discussion of the issue and its impact in the
<trollbait>upcoming version of VB</trollbait>, read:

http://www.panopticoncentral.net/archive/2006/03/28/11552.aspx

Regards,

Branco.
 
R

RobinS

Branco Medeiros said:
<snip>

Nope. This is allways a source of misunderstanding, even though it's
well documented.
If you use block variables, initialize them (unless you want such side
effects, as preserved values between cycles).

See:
http://msdn2.microsoft.com/en-us/library/1t0wsc67.aspx

For a lengthier discussion of the issue and its impact in the
<trollbait>upcoming version of VB</trollbait>, read:

http://www.panopticoncentral.net/archive/2006/03/28/11552.aspx

Regards,

Branco.

Thanks. The trollbait tags made me laugh out loud after
a particularly difficult day.

Robin S.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top