Variable inside or outside of loop?

This topic is 3493 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

Recommended Posts

        // OVER HERE
Dim fileName As String
For i = 0 To CheckBoxList1.Items.Count - 1
If CheckBoxList1.Items(i).Selected Then
'write function to do copy
fileName = CheckBoxList1.Items(i).Text
If Not CopyFiles(fileName) Then
Return
End If
End If
Next
// or
For i = 0 To CheckBoxList1.Items.Count - 1
If CheckBoxList1.Items(i).Selected Then
'write function to do copy
// OVER HERE
Dim fileName As String = CheckBoxList1.Items(i).Text
If Not CopyFiles(fileName) Then
Return
End If
End If
Next


Share on other sites
Do you mean which is better performance-wise? Because I don't think that would matter at all.

However, why not do away with the string entirely?

For i = 0 To CheckBoxList1.Items.Count - 1    If CheckBoxList1.Items(i).Selected Then        'write function to do copy        If Not CopyFiles(CheckBoxList1.Items(i).Text) Then            Return        End If    End IfNext

Share on other sites
Inside when you can, outside when you need to, is the general rule: restrict variable scope as far as possible. That way avoids collisions, and puts declaration near use, where it's more easily looked for. Of course, this doesn't matter so much for languages that don't introduce a new variable scope with each control structure, and/or don't use explicit declarations (Python falls in both categories :) ).

Anyway, doesn't that language (VB?) have any kind of foreach loop? Why are you iterating over a sequence of numbers only to use them to index back into a container, as opposed to actually iterating over elements of the container? :( (Even if you're stuck with that, the best use of a variable in your code is not to identify the filename, but to store the current item for iteration. After all, as it stands, the phrase 'CheckBoxList1.Items(i)' gets repeated in your code, but 'CheckBoxList1.Items(i).Text' does not.)

Share on other sites
I am not sure what the question is asking.
Is it a scope question?

Share on other sites
Dim ListItem as VariantFor Each ListItem in CheckBoxList1.Items  If ListItem.Selected And Not CopyFiles(ListItem.Text) Then    Return  End IfNext

Share on other sites
A very slight variation on what Oluseyi did (seeing as it looks like VB.NET):
' I'm assuming you're using a listview here due to the 'Text' property, if not, change it.For Each item as ListViewItem in CheckBoxList1.Items   If ((item.Selected) AndAlso Not (CopyFiles(item.Text)) then     Return   End IfNext item

Declaring the loop variable in the loop itself is just a personal preference (and I think is required in C#). I also prefer to use AndAlso (or its counterpart, OrElse) because it allows for short circuit evaluation. Of course short circuit eval may not have the effect you're looking for (i.e. CopyFiles won't run when item.Selected = False), but I try to use it whereever possible.