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

7 Dec 2006 3:34 PM

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

7 Dec 2006 6:40 PM

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.

-----
Tim Patrick - www.timaki.com
Start-to-Finish Visual Basic 2005

Show quoteHide quote

> 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

7 Dec 2006 6:47 PM

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.

<teddysn***@hotmail.com> wrote in message
Show quoteHide quote

news:1165505686.265584.3330@f1g2000cwa.googlegroups.com...
> 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
>

8 Dec 2006 8:46 AM

teddysnips

Marina Levit [MVP] wrote:

> 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

8 Dec 2006 5:51 PM

Branco Medeiros

teddysn***@hotmail.com wrote:

> Is this a bug?

<snip>

> Do While dr.Read
>     Dim ClubDay As New ChildrensClubDay
>     With ClubDay
>
>         Dim MPInfo As PaymentInformation

<snip>

> 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.

9 Dec 2006 3:21 AM

RobinS

Show quote Hide quote

"Branco Medeiros" <branco.medei***@gmail.com> wrote in message
news:1165600272.242952.264660@j44g2000cwa.googlegroups.com...
>
> teddysn***@hotmail.com wrote:
>> Is this a bug?
> <snip>
>> Do While dr.Read
>> Dim ClubDay As New ChildrensClubDay
>> With ClubDay
>>
>> Dim MPInfo As PaymentInformation
> <snip>
>> 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.
>

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

Robin S.