[Ginister]

Hi everybody,
I've put a list view in my program which operates fine, but if I check two items, they aren't removed from the file. I can't seem to understand why as I have used a for each to remove them. Here is the code snippet.

Private Sub btnUnbook_Click(sender As System.Object, e As System.EventArgs) Handles btnUnbook.Click
        Dim IndexOfChoice As String 'Index of the item a user has chosen
        Dim MakeNewFile As New System.IO.StreamWriter(CurDir() & "\NewFile.csv") 'Make new file is a writer used to write every line apart from the unbooked facilities
        Dim CollectOldFile As New System.IO.StreamReader(PCFile) 'Collect old file is a reader

        Dim FileLineText As String 'File line text holds the current line
        Dim CurrentIndex As Integer 'Current index is the item that the program is checking

        For Each SelectedItems As ListViewItem In lstFacilities.CheckedItems 'For each checked item (Dimmed as Selected item to make it simpler)
            IndexOfChoice = SelectedItems.Index 'Index of selected item is put into a variable

            lstFacilities.Items.Remove(SelectedItems) 'Remove current item from display

            FileLineText = "" 'Blank file line text

            CollectOldFile.Close() 'Closes file reader and writer to avoid a second choice crashing the program
            MakeNewFile.Close()

            CollectOldFile = New IO.StreamReader(PCFile) 'Declares the writer and reader as new reader/writers
            MakeNewFile = New IO.StreamWriter(CurDir() & "\NewFile.csv")


            Do 'Begins a do loop
                FileLineText = CollectOldFile.ReadLine 'Hold read line in a variable
                If FileLineText = Nothing Then 'If nothing found
                    Exit Do


                Else 'Otherwise

                    If CurrentIndex <> IndexOfChoice Then 'If this is not the selected item
                        MakeNewFile.WriteLine(FileLineText) 'Write to new file

                    End If

                End If
                CurrentIndex += 1 'Increment the index

            Loop Until FileLineText = Nothing

            CollectOldFile.Close() 'Close reader and writer
            MakeNewFile.Close()

            System.IO.File.Delete(PCFile) 'Delete outdated file
            My.Computer.FileSystem.RenameFile(CurDir() & "\NewFile.csv", "PCFile.csv") 'Rename as PC file


        Next


        MsgBox("The facilitiy selected has been unbooked.", , "Unbooking Successful") 'Inform the user

    End Sub

In addition, how do you make a message box appear if there are no checked items?
Thanks,
Bryn.

[andrewsw]

The main problem is that you are using a For Each to loop through the checked-items, but removing items within this loop. This will cause problems as the items are constantly re-indexed as you remove them.

Use code like the following to remove all checked items:

        While lstFacilities.CheckedItems.Count > 0
            lstFacilities.Items.Remove(lstFacilities.CheckedItems(0))
        End While

I would put this at the bottom of your procedure (if suitable) to do after everything else has been completed.

Q2:

If lstFacilities.CheckedItems.Count > 0 Then
    MessageBox.Show("You did not check anything!")
End If

BTW You are instantiating your StreamReader and StreamWriter twice.

[Ginister]

andrewsw, on 17 February 2013 - 11:10 AM, said:

The main problem is that you are using a For Each to loop through the checked-items, but removing items within this loop. This will cause problems as the items are constantly re-indexed as you remove them.

Use code like the following to remove all checked items:

        While lstFacilities.CheckedItems.Count > 0
            lstFacilities.Items.Remove(lstFacilities.CheckedItems(0))
        End While

I would put this at the bottom of your procedure (if suitable) to do after everything else has been completed.

Q2:

If lstFacilities.CheckedItems.Count > 0 Then
    MessageBox.Show("You did not check anything!")
End If

BTW You are instantiating your StreamReader and StreamWriter twice.

Thank you, that is a great help to me. However, I have just realised that the list view only displays entries booked by the current user, so the file will not have the correct index from using the list view's index value. What would be a good solution to this?

[andrewsw]

Quote

so the file will not have the correct index from using the list view's index value

Erm not sure what file you are referring to. Perhaps you have a list of files stored in a file that you are reading from? Anyway, you'll probably need to loop through your collection (list, or whatever) comparing each filename to the selected one(s). Or, you could lookup the item, rather than iterating them all. It depends..

[I'd rather you explain your overall process rather than me trying to interpret it from your code.]

This post has been edited by andrewsw: 17 February 2013 - 03:28 PM

[Ginister]

andrewsw, on 17 February 2013 - 03:22 PM, said:

Quote

so the file will not have the correct index from using the list view's index value

Erm not sure what file you are referring to. Perhaps you have a list of files stored in a file that you are reading from? Anyway, you'll probably need to loop through your collection (list, or whatever) comparing each filename to the selected one(s). Or, you could lookup the item, rather than iterating them all. It depends..

[I'd rather you explain your overall process rather than me trying to interpret it from your code.]

Yeah, sorry, just realised that was a bit vague! I'm saving the data entered on a different form into a file. This current form unbooks them, i.e. removes the line of text from the file. This is done here by making a new file that does not have that text in and renaming it. But the issue here is that the items in the list view don't match up to the items in the file, so the index can't be used.

[andrewsw]

A father-son update. Here's some code that demonstrates in principle:

Dim _sr As New StreamReader(filename)
Dim _sw As New StreamWriter(Path.ChangeExtension(filename, "tmp"))
Dim _buffer As String = _sr.ReadLine()
While _buffer IsNot Nothing
    _sw.WriteLine(_buffer.Replace("something", "somethingelse"))
    _buffer _sr.ReadLine()
End While
_sr.Close();
_sw.Close();
File.Copy(Path.ChangeExtension(filename, "tmp"), filename, True)
File.Delete(Path.ChangeExtension(filename, "tmp"))
Catch _ex As Exception
ErrLog(_ex)
End Try

Basically, it is using StreamReader and Writer in parallel, but only writing required content to the other file. Then it renames the new file and deletes the temporary version.

So, as you read each line, check whether it contains the text from the ListView - if so, don't include it in the new file.

If the files are small then you might ReadAllLines() into a list and loop through these (removing items/lines) then write them to another file.

[andrewsw]

Yes, if the file is small then ReadAllLines is easier: MSDN and an example from that page:

Imports System
Imports System.IO

Public Class Test
    Public Shared Sub Main()
        Dim path As String = "c:\temp\MyTest.txt" 

        ' This text is added only once to the file. 
        If File.Exists(path) = False Then 

            ' Create a file to write to. 
            Dim createText() As String = {"Hello", "And", "Welcome"}
            File.WriteAllLines(path, createText)
        End If 

        ' This text is always added, making the file longer over time 
        ' if it is not deleted. 
        Dim appendText As String = "This is extra text" + Environment.NewLine
        File.AppendAllText(path, appendText)

        ' Open the file to read from. 
        Dim readText() As String = File.ReadAllLines(path)
        Dim s As String 
        For Each s In readText
            Console.WriteLine(s)
        Next 
    End Sub 
End Class

[Ginister]

andrewsw, on 17 February 2013 - 04:49 PM, said:

Yes, if the file is small then ReadAllLines is easier: MSDN and an example from that page:

Imports System
Imports System.IO

Public Class Test
    Public Shared Sub Main()
        Dim path As String = "c:\temp\MyTest.txt" 

        ' This text is added only once to the file. 
        If File.Exists(path) = False Then 

            ' Create a file to write to. 
            Dim createText() As String = {"Hello", "And", "Welcome"}
            File.WriteAllLines(path, createText)
        End If 

        ' This text is always added, making the file longer over time 
        ' if it is not deleted. 
        Dim appendText As String = "This is extra text" + Environment.NewLine
        File.AppendAllText(path, appendText)

        ' Open the file to read from. 
        Dim readText() As String = File.ReadAllLines(path)
        Dim s As String 
        For Each s In readText
            Console.WriteLine(s)
        Next 
    End Sub 
End Class

ReadAllLines isn't really ideal for my file I think, but thanks for the idea My issue is really with checking the info and matching it against the right value in the file. What I've done is, because indexes aren't consistent with the list view, I've made it now check each item's text, which works quite well for when only one item is checked. It's fixed the original issue, but now I have a different one. See below.

For i = 0 To lstFacilities.CheckedItems.Count - 1
                    SelectedItems.Selected = True
                    FileChecker(0) = lstFacilities.SelectedItems(i).SubItems(0).Text - 1 'Sets the values in an array to the relevant subitem at that number
                    FileChecker(1) = lstFacilities.SelectedItems(i).SubItems(1).Text - 1 'For each selected item, i
                    FileChecker(2) = lstFacilities.SelectedItems(i).SubItems(2).Text
                    FileChecker(3) = lstFacilities.SelectedItems(i).SubItems(3).Text
                    FileChecker(4) = lstFacilities.SelectedItems(i).SubItems(4).Text
                    FileChecker(5) = lstFacilities.SelectedItems(i).SubItems(5).Text
                    FileChecker(6) = lstFacilities.SelectedItems(i).SubItems(6).Text

                    FileCheckText = FileChecker(0) & "," & FileChecker(1) & "," & FileChecker(2) & "," & FileChecker(3) & "," & FileChecker(4) & "," & FileChecker(5) & "," & FileChecker(6)





                    FileLineText = "" 'Blank file line text


                    Do 'Begins a do loop
                        FileLineText = CollectOldFile.ReadLine 'Hold read line in a variable
                        If FileLineText = Nothing Then 'If nothing found
                            Exit Do

                        Else 'Otherwise

                            If FileCheckText <> FileLineText Then
                                MakeNewFile.WriteLine(FileLineText) 'Write to new file

                            End If

                        End If


                    Loop Until FileLineText = Nothing











                    CollectOldFile.Close() 'Close reader and writer
                    MakeNewFile.Close()





                    System.IO.File.Delete(PCFile) 'Delete outdated file
                    My.Computer.FileSystem.RenameFile(CurDir() & "\NewFile.csv", "PCFile.csv") 'Rename as PC file






                    lstFacilities.Items.Remove(SelectedItems) 'Remove current item from display
                    MsgBox("The facilitiy selected has been unbooked.", , "Unbooking Successful") 'Inform the user

                Next
            Next

I'm getting the error "InvalidArgument=Value of '1' is not valid for 'index'." when I select multiple items to be removed. This occurs on the assignment for Filechecker. Not sure why since I have set it to count - 1. Please help!

[andrewsw]

FileChecker(0) = lstFacilities.SelectedItems(i).SubItems(0).Text - 1

Why are you attempting to subtract 1 from a string?

A quick glance at your code tells me that you are still attempting to remove list-items within a loop that references the listitems, which is a problem as pointed out earlier. "Invalid Argument.." is probably confirming this problem, because you are trying to refer to an item which has been removed.

[Ginister]

andrewsw, on 18 February 2013 - 12:02 PM, said:

FileChecker(0) = lstFacilities.SelectedItems(i).SubItems(0).Text - 1

Why are you attempting to subtract 1 from a string?

A quick glance at your code tells me that you are still attempting to remove list-items within a loop that references the listitems, which is a problem as pointed out earlier. "Invalid Argument.." is probably confirming this problem, because you are trying to refer to an item which has been removed.

Yeah, that's supposed to happen, don't worry too much It's a coordinate for the first two, but since it's 0-based I've added 1 so the user can see what it is in a 1-based system which is what they're more familiar with. So to get the coordinates back to usable figures, I'm taking away that 1. Suppose I should be using a CInt, but seems to work thus far!

Could you explain in layman's terms your point about the referencing? I don't fully understand! :S
Thanks again.

[Ginister]

For reference, if I delete the remove code it still pops the same error.

[andrewsw]

Ginister, on 18 February 2013 - 01:20 PM, said:

Could you explain in layman's terms your point about the referencing? I don't fully understand!

It may not be the issue with your current code but, my rough and ready description is:

Consider the CheckedItems from your earlier post. These are indexed starting at 0. So four checked items will be item(0), 1, 2 and 3. If you use a For Each loop to loop through these items and, in the first iteration, you remove the first item, then the CheckedItems collection will now be Item(0), 1 and 2, where Item(0) is the item formerly known as Item(1), etc. Notice that there will now no longer be an Item(3), so attempting to refer to this item will cause an error. But also, what you thought was Item(2) is now Item(1).

Removing ListView items doesn't work in the same way as they are not automatically re-indexed after every removal.

As you can see, not every collection behaves in the same way, but it is important to be aware of the different behaviours. The safest way to remove items from a collection (within a loop) is generally:

Dim collTotal As Integer = MyCollection.Count
For x As Integer = collTotal-1 To 0 Step -1
    MyCollectionRemoveAt(x)
Next

This way, items are removed from the end and any re-indexing won't cause problems, because the earlier (lower-indexed) items will not change their index number.

Anyway, this is just a rough-and-ready description (I haven't checked all the details).

**********
You need to debug your code to discover the problem. Refer to the link in my signature.

This post has been edited by andrewsw: 18 February 2013 - 02:22 PM

[andrewsw]

Anyway, it is not advisable to attempt to remove items from a collection within a For..Each loop.

[Ginister]

Bingo, I understand what you mean now. This was the issue as the lstFacilities count was incorrect when something was removed, thus causing the above error. Thank you for your help!