Better way to implement removing an item from a List

[BobRoyAce]

I have a class which has a private variable as follows (where
EntityAttachedDocument is another class in the project):

Private _EntityAttachedDocuments As List(Of EntityAttachedDocument)

In addition, I have two methods as follows:

Public Sub RemoveDocumentsToBeRemoved(ByVal sAttachdDocumentsFolder As
String)
For Each oDoc As EntityAttachedDocument In _EntityAttachedDocuments
If (oDoc.IsToBeRemoved = True) Then
RemoveEntityAttachedDocument(oDoc, sAttachdDocumentsFolder)
End If
Next
End Sub

Public Sub RemoveEntityAttachedDocument( _
ByVal oEntityAttachedDocumentToRemove As EntityAttachedDocument, _
ByVal sAttachedDocsFolder As String)
Dim sFilenameToRemove As String

For Each oEntityAttachedDocument As EntityAttachedDocument In
EntityAttachedDocuments
If (oEntityAttachedDocument.UniqueID =
oEntityAttachedDocumentToRemove.UniqueID) Then
sFilenameToRemove = oEntityAttachedDocument.DocumentFilename

EntityAttachedDocuments.Remove(oEntityAttachedDocument)

' If we're not dealing with a NEW Attached Doc, then delete the
corresponding EntityAttachedDocument record
If (oEntityAttachedDocument.IsNew = False) Then
oEntityAttachedDocument.Delete()
End If

' Only attempt to delete the document if 1) sAttachedDocsFolder
is not blank, 2) document actually exists,
' and 3) IsNew = False (since, if IsNew = True, document has not
been copied to the folder yet).
If (sAttachedDocsFolder.Length > 0) AndAlso
(_RemovedDocumentsShouldBeDeleted = True) AndAlso
(oEntityAttachedDocument.IsNew = False) Then
Try
If (My.Computer.FileSystem.FileExists(sAttachedDocsFolder &
sFilenameToRemove)) Then
My.Computer.FileSystem.DeleteFile(sAttachedDocsFolder &
sFilenameToRemove, FileIO.UIOption.OnlyErrorDialogs,
FileIO.RecycleOption.SendToRecycleBin)
End If
Catch ex As Exception
'...
End Try
Else
'...
End If
End If
Next

' If we got here, then we never found the EntityAttachedDocument, so
couldn't remove...
End Sub

The first method is attempting to remove from the
EntityAttachedDocument list all EntityAttachedDocument items
whose IsToBeRemoved property is true. To actually perform the removal,
the RemoveDocumentsToBeRemoved method
calls the RemoveEntityAttachedDocument method.

Well, this results in a runtime error (related to "collection was
modified") since the RemoveEntityAttachedDocument
method ends up removing an item from the list that
RemoveDocumentsToBeRemoved was iterating through. Now, I'm sure
that one solution would be to copy the code from the inside of the
For...Next loop in RemoveEntityAttachedDocument
and reproduce it in the For...Next loop in RemoveDocumentsToBeRemoved.
However, I don't like that and I'm thinking that there has to be a
better way.

Any suggestions?

 

[SurturZ]

Use a positional index rather than For..Each and go backwards instead.

For i as Integer = (_EntityAttachedDocuments.Count - 1) to 0 Step -1
Dim oDoc As EntityAttachedDocument = _EntityAttachedDocuments(i)
if oDoc..IsToBeRemoved = True) Then
RemoveEntityAttachedDocument(oDoc, sAttachdDocumentsFolder)
End If
Next i

You might need to rewrite RemoveEntityAttachedDocument to use .RemoveAt
rather than .Remove, not sure.

 

[Guest]

Well, this results in a runtime error (related to "collection was
modified") since the RemoveEntityAttachedDocument
method ends up removing an item from the list that
RemoveDocumentsToBeRemoved was iterating through. Now, I'm sure
that one solution would be to copy the code from the inside of the
For...Next loop in RemoveEntityAttachedDocument
and reproduce it in the For...Next loop in RemoveDocumentsToBeRemoved.
However, I don't like that and I'm thinking that there has to be a
better way.

Iterate backwards, and remove items by index. Don't use a For Each Loop but
rather a regular For loop.

 

[Cor Ligthert[MVP]]

Rob,

Any suggestions?

Yea, make yourself a simple sample to test what you are doing with simple
datanames.
Probably that shows you the problem in a quick way, if not then you have
something to show us youe problem for a newsgroup more suitable way.

Cor