CheckedListBox Problems

B

bjwade62

Centurion
Joined
Oct 31, 2003
Messages
104
I have two problems with my code for a CheckedListBox. I have a sub that checks to see if an item in a CheckedListBox is checked. If it is it's supposed to copy the corrisponding file to a folder. See code below.

Problem #1
I get an IndexOutOfRangeException error at line:
clbText = CheckedListBox1.CheckedItems.Item(i)

Problem # 2
After the file is copied I want the checked item to become unchecked.

Here's my code. Thanks.

Visual Basic: 
        If Me.CheckedListBox1.CheckedItems.Count > 1 Then
            Dim clbText As String
            Dim i As Integer
            For i = 0 To CheckedListBox1.Items.Count - 1
                If Me.CheckedListBox1.GetItemCheckState(i) Then
                    clbText = CheckedListBox1.CheckedItems.Item(i)
                    'copies selected file to project folder
                    Dim strFileToSaveNoPath As String = System.IO.Path.GetFileNameWithoutExtension(clbText)
                    Dim strFileToSaveNoPathDWG As String = strFileToSaveNoPath & ".dwg"
                    strFileToSave = Me.dirStdDtl.Path & "\" & strFileToSaveNoPath & ".dwg"
                    Dim strNewFile As String = Me.proj_dir.Path & "\" & strFileToSaveNoPathDWG
                    Dim FileExists As Boolean = System.IO.File.Exists(strNewFile)

                    If FileExists = True Then
                        Dim msg As String = ("One or more detail(s) already exist in " & Me.proj_dir.Path & "." _
                               & vbCrLf & "Are you sure you want overwrite existing detail(s)?")
                        Dim Response As String = MsgBox(msg, MsgBoxStyle.OkCancel, "File(s) already exists")
                        Select Case Response
                            Case vbOK
                                IO.File.Delete(strNewFile)
                                System.IO.File.Copy(strFileToSave, strNewFile)
                            Case vbCancel
                                Exit Sub
                        End Select
                    Else
                        System.IO.File.Copy(strFileToSave, strNewFile)
                    End If
                End If

                Me.CheckedListBox1.SetItemCheckState(i, CheckState.Unchecked)
                Me.CheckedListBox1.Refresh()
            Next i
        End If
 
I'm guessing that you are switching over from VB6. For starters, I recommend you keep option strict on. It will catch certain errors before you even compile. For instance, for your fifth line you are implicitly converting a CheckState value into a boolean which could result in undesirable behavior (in fact, you have this kind of problem in a few places in your code).

The reason you are getting a bounds error is because on the 6th line you are accessing items in CheckedListBox1.CheckedItems using an index based on all the items. When you get to item #3 you looking at CheckedItems(2) when you should be looking it Items(2).

Visual Basic: 
' This variable tracks if the user has okayed overwriting old files so we only need to ask once.
Dim Overwrite As Boolean = False
 
If Me.CheckedListBox1.CheckedItems.Count > 1 Then
    For i As Integer = 0 To CheckedListBox1.Items.Count - 1
        ' You should explicitly compare the CheckState value to what you expect
        If Me.CheckedListBox1.GetItemCheckState(i) = CheckState.Checked Then
            ' Try to use clear meaningful variable names. When you are using a strongly typed
            '    language with intellisense there is very little need for hungarian naming conventions
            '    (i.e. starting variable names with "str," "int," etc.
            ' ListBoxes store items as System.Objects. You should convert them back to strings.
            ' Since the looping variable (i) is based on Items.Count and not CheckedItems.Count, you should be accessing items
            '    using the Items(i) property instead of the CheckedItems(i) property.
            Dim OldFilename As String = CheckedListBox1.Items(i).ToString()
 
            ' Determine new filename. Not sure exactly how you constructed the filename
            ' but you should make more use of the Path class to guaruntee file path manipulations
            ' are correct and the code is more readable.
            Dim NewFilename As String = Path.Combine( _
                Path.Combine(Me.proj_dir.Path, dirStdDtl.Path), _
                Path.GetFileNameWithoutExtension(OldFilename) & ".dwg")
 
            ' I rewrote this whole part of the code. It is clearer to me now but you should do what makes sense to
            ' you when it comes to the logical layout of code.
            If File.Exists(NewFilename) Then
                If (Overwrite) Then
                    ' If the user has already okayed overwriting files just delete the old one.
                    File.Delete(NewFilename)
                Else
                    ' Otherwise ask the user.
                    ' The "proper" .Net way of using message boxes is with the MessageBox class.
                    ' Also, neither MsgBox() nor MessageBox.Show() return a string. This is another
                    '     example of where option strict would help.
                    Dim Response As DialogResult = _
                        MessageBox.Show("One or more detail(s) already exist in " & Me.proj_dir.Path & "." _
                           & Environment.NewLine & "Are you sure you want overwrite existing detail(s)?", _
                           "File(s) already exist.", _
                           MessageBoxButtons.OKCancel)
 
                    'Based on the response, either delete old file or abort the operation
                    Select Case Response
                        Case Windows.Forms.DialogResult.OK
                            File.Delete(NewFilename)
                            Overwrite = True 'User has okayed overwrite, don't ask again
                        Case Windows.Forms.DialogResult.Cancel
                            Return ' Just FYI. Same as 'Exit Sub'
                    End Select
                End If
            End If
 
            ' Copy the file
            System.IO.File.Copy(OldFilename, NewFilename)
            ' Clear the check
            Me.CheckedListBox1.SetItemCheckState(i, CheckState.Unchecked)
        End If
    Next i
End If
There is a lot of criticism in there, but hopefully it helps your transition to VB .Net.
 
I'm such a rookie at .NET that I welcome criticism. That looks like it took a lot of your time so thanks tons. I'm at home right now but I can't wait to get to the office tomorrow and try your new code. I really appreciate the time you took to comment it too. Sure I could cut and paste your code, but unless understand what you did I've learned nothing.

Again many thanks to you Marble_Eater!

Bernie

marble_eater said:
I'm guessing that you are switching over from VB6. For starters, I recommend you keep option strict on. It will catch certain errors before you even compile. For instance, for your fifth line you are implicitly converting a CheckState value into a boolean which could result in undesirable behavior (in fact, you have this kind of problem in a few places in your code).

The reason you are getting a bounds error is because on the 6th line you are accessing items in CheckedListBox1.CheckedItems using an index based on all the items. When you get to item #3 you looking at CheckedItems(2) when you should be looking it Items(2).

Visual Basic: 
' This variable tracks if the user has okayed overwriting old files so we only need to ask once.
Dim Overwrite As Boolean = False
 
If Me.CheckedListBox1.CheckedItems.Count > 1 Then
    For i As Integer = 0 To CheckedListBox1.Items.Count - 1
        ' You should explicitly compare the CheckState value to what you expect
        If Me.CheckedListBox1.GetItemCheckState(i) = CheckState.Checked Then
            ' Try to use clear meaningful variable names. When you are using a strongly typed
            '    language with intellisense there is very little need for hungarian naming conventions
            '    (i.e. starting variable names with "str," "int," etc.
            ' ListBoxes store items as System.Objects. You should convert them back to strings.
            ' Since the looping variable (i) is based on Items.Count and not CheckedItems.Count, you should be accessing items
            '    using the Items(i) property instead of the CheckedItems(i) property.
            Dim OldFilename As String = CheckedListBox1.Items(i).ToString()
 
            ' Determine new filename. Not sure exactly how you constructed the filename
            ' but you should make more use of the Path class to guaruntee file path manipulations
            ' are correct and the code is more readable.
            Dim NewFilename As String = Path.Combine( _
                Path.Combine(Me.proj_dir.Path, dirStdDtl.Path), _
                Path.GetFileNameWithoutExtension(OldFilename) & ".dwg")
 
            ' I rewrote this whole part of the code. It is clearer to me now but you should do what makes sense to
            ' you when it comes to the logical layout of code.
            If File.Exists(NewFilename) Then
                If (Overwrite) Then
                    ' If the user has already okayed overwriting files just delete the old one.
                    File.Delete(NewFilename)
                Else
                    ' Otherwise ask the user.
                    ' The "proper" .Net way of using message boxes is with the MessageBox class.
                    ' Also, neither MsgBox() nor MessageBox.Show() return a string. This is another
                    '     example of where option strict would help.
                    Dim Response As DialogResult = _
                        MessageBox.Show("One or more detail(s) already exist in " & Me.proj_dir.Path & "." _
                           & Environment.NewLine & "Are you sure you want overwrite existing detail(s)?", _
                           "File(s) already exist.", _
                           MessageBoxButtons.OKCancel)
 
                    'Based on the response, either delete old file or abort the operation
                    Select Case Response
                        Case Windows.Forms.DialogResult.OK
                            File.Delete(NewFilename)
                            Overwrite = True 'User has okayed overwrite, don't ask again
                        Case Windows.Forms.DialogResult.Cancel
                            Return ' Just FYI. Same as 'Exit Sub'
                    End Select
                End If
            End If
 
            ' Copy the file
            System.IO.File.Copy(OldFilename, NewFilename)
            ' Clear the check
            Me.CheckedListBox1.SetItemCheckState(i, CheckState.Unchecked)
        End If
    Next i
End If
There is a lot of criticism in there, but hopefully it helps your transition to VB .Net.
Click to expand...
 
Hey Marble_eater!

I had the same question on another forum. Your answer works great, but this one has less code. I'm passing it on to you in case you weren't aware of it. The particular section that I'm talking about is the FileCopy section.

The UIOption.AllDialogs replaces the need for additonal code to overwrite existing files. Please don't get the worng idea. You know far more about VB.NET than I'll ever know. I'm just trying to pass on some information to someone who's helped me out in the past. Thanks again for all your help.

Bernie


Visual Basic: 
'Yours
System.IO.File.Copy(OldFilename, NewFilename)

'Other Forum
Imports Microsoft.VisualBasic.FileIO
FileSystem.CopyFile(SourceFile, TargetFile, UIOption.AllDialogs)
 
Don't ever think that I'm too proud to learn something from a less experienced programmer, but I'm a little confused. What is the advantage of the second listing of code you posted? And just for the record, I gave VB up about a year ago in favor of C# because I like the syntax more and because VB seems to be separating itself from the .Net mainstream (for instance, the large number of VB-specific functions and the "My" feature). I can carry almost anything over from C# to VB, but vbCrLf and My.FileSystem just don't translate into C# so easily. Oh, and please stop cheating on us. It hurts us inside.
 
The second listing (FileSystem.CopyFile(SourceFile, TargetFile, UIOption.AllDialogs, UICancelOption.DoNothing)) eliminates the need to create a messagebox to handle finding out if the file already exists. It also overwrites it if it does. So I was able to remove all the code that handled that previously.

Thanks again.... I'll try to be monogamous in the future. :)

marble_eater said:
Don't ever think that I'm too proud to learn something from a less experienced programmer, but I'm a little confused. What is the advantage of the second listing of code you posted? And just for the record, I gave VB up about a year ago in favor of C# because I like the syntax more and because VB seems to be separating itself from the .Net mainstream (for instance, the large number of VB-specific functions and the "My" feature). I can carry almost anything over from C# to VB, but vbCrLf and My.FileSystem just don't translate into C# so easily. Oh, and please stop cheating on us. It hurts us inside.
Click to expand...
 
Back
Top