Iterating through arraylist and removing an object

Question

Title says it all, I'm having some trouble iterating through an arraylist I've made (songs) and removing a specific song once it's been found. I've confirmed that the locations are correct and have tried manually entering in a song's location to no avail.

    Sub delete(location As String)
    Dim tempS As Song
    For Each tempS In songs
        If tempS.getLocation().toLower().Equals(location) Then
            songs.Remove(tempS)
        End If
        Exit For
    Next
End Sub

This project is still very much in its beginning stage and the only thing a song has associated with itself is it's stored location (ei C:\Music\Albums\Once\Nemo.mp3). Many thanks!

is the location in .equals(location) also to lower? If it isn't, than it won't match. — Kat, Aug 26 '14 at 15:40
I think you want that `Exit For` inside the `If` block. Be careful when iterating through any array and deleting items during iteration as you can't always be sure what you are iterating after you delete one of them. — Matt Wilko, Aug 26 '14 at 15:40
it'll be C:\Music\Albums\Once\Nemo.mp3 versus c:\music\albums\once\nemo.mp3 — Kat, Aug 26 '14 at 15:40
Thank you both! Moving the Exit For did the trick, can't believe it was something that simple! Haha yes my bad I forgot to mention that all songs are converted to lower before being added to the arraylist. — Novastorm, Aug 26 '14 at 15:46

Matt Wilko · Accepted Answer · 2014-08-26T15:57:55.390

There are a couple of issues in your code.

Firstly you are not comparing the same case because location does not have a .ToLower() after it.

Secondly you are potentially removing an item from your list whilst still iterating through it. This can cause issues.

You can improve this considerably by using linq. That way you can do it all in one line:

Public Sub DeleteByLocation(location As String)
    songs.RemoveAll(Function(x) x.GetLocation().Equals(location, StringComparison.OrdinalIgnoreCase))
End Sub

Change RemoveAll to Remove to improve performance if you are certain that there will only be one matching location

Thank you Matt, to the point and does exactly what I need with less code. — Novastorm, Aug 26 '14 at 15:57

Iterating through arraylist and removing an object

1 Answers1