Having problems reading from and deleting certain

Question

The point of this program is to remove certain sports teams and their members from a text file, then overwrite the original file with the new set of values. This is attempted by reading the values into an array, then looping through the array and deleting the name of the team and the next 2 lines but for some reason it stops going through the array after the index.

I'm stuck, so any help would be great

Code:

private void RemoveSportsTeamButtonActionPerformed(java.awt.event.ActionEvent evt) {

    String ChosenTeam = "";
    ChosenTeam = JOptionPane.showInputDialog("What Team Do you want to remove?");

    ArrayList<String> Teamlist = new ArrayList<String>();

    if (ChosenTeam.length() > 0) {
    } else {
        Scanner Reader = null;
        try {
            Reader = new Scanner(new File("ListofSportTeams.txt"));
        } catch (FileNotFoundException ex) {

        }

        while (Reader.hasNext()) {
            Teamlist.add(Reader.next());
        }
        Reader.close();

        for (int count = 0; count < Teamlist.size(); count++) {
            {
                if (Teamlist.get(count).equals(ChosenTeam)) {
                    Teamlist.remove(count);
                    Teamlist.remove(count + 1);
                    Teamlist.remove(count + 2);
                }
            }

        }
    }

}

SportTeamList.txt =

Team1
Jeff
James
Team2
Steve
Peter

Answer 1

Now, I am using two lists. One is the original, the other is the removal list. I read the original list, if an entry starts with "Team" (please add if you have some other logic for differentiating team names from member names), I am adding it to the removal list and also the following entries until the next team name to be retained is found. Finally, I remove all the removal entries from the original list.

public class ListRemovalDemo 
{
    public static void main(String[] args) 
    {
            List<String> teamList = new ArrayList<String>();
            teamList.add("TeamName1");
            teamList.add("Member1Team1");
            teamList.add("Member2Team1");
            teamList.add("TeamName2");
            teamList.add("Member1Team2");
            teamList.add("Member2Team2");
            teamList.add("TeamName3");
            teamList.add("Member1Team3");
            teamList.add("Member2Team3");
            List<String> removalList = new ArrayList<String>();
            String teamToRemove = "TeamName2";
            Iterator<String> teamListIterator = teamList.listIterator();
            String entry;
            while(teamListIterator.hasNext())
            {
                entry = teamListIterator.next();
                if(entry.equals(teamToRemove))
                {
                    removalList.add(entry);  
                    if(teamListIterator.hasNext())
                    {
                        entry = teamListIterator.next();
                            while(!entry.startsWith("Team"))
                            {
                                removalList.add(entry);
                                if(teamListIterator.hasNext())
                                    entry = teamListIterator.next();
                                else
                                    break;
                            }
                    }
                }
            }
            teamList.removeAll(removalList);
            System.out.println("After removal of " + teamToRemove + ":\n" + teamList);
    }
}

output

After removal of TeamName2:
[TeamName1, Member1Team1, Member2Team1, TeamName3, Member1Team3, Member2Team3]

Answer 2

You should never remove from a List when iterating. Which you are doing.

Consider a trivial example, I have a list {1,2,3,4,5}. Let us assume its 0-indexed and I want to remove all numbers greater than 3.

0 - List item is 1, keep
1 - List item is 2, keep
2 - List item is 3, remove. All elements get shifted, list is now {1,2,4,5}.
3 - List item is 5, remove
4 - List item is oops, there is no longer a 4

So I have overshot the end of the List because I took the size to be 5 when I started iterating but it became 4 after I removed the element at index 2 and it became 3 when I removed the element at index 3.

You might say, "Ah ha, I can fix this with a while loop":

int i = 1;
while(i < teams.size()) {
    //do stuff
}

But this is even worse:

0 - List item is 1, keep
1 - List item is 2, keep
2 - List item is 3, remove. All elements get shifted, list is now {1,2,4,5}.
3 - List item is 5, remove

So, no error. Looks like the issue is fixed. But what does the list contain? It contains {1,2,4}. But 4 is greater than 3. It was skipped due to the index shift. You now have an even more insidious bug.

If you were using a proper enhanced foreach loop like so:

for(final String team : teams) {
    //do stuff with team
}

You would have, correctly, gotten a ConcurrentModificationException. This is just one of many reasons to use the enhanced foreach loop rather than looping by index.

In order to do what you want use an Iterator:

final Iterator<String> iter = teams.iterator();
while(iter.hasNext()) {
    if(iter.next().equals(testString))
        iter.remove();
}

I will reiterate my comment:

Please always use Java naming conventions. Variables should always be in camelCase. PascalCase is reserved for classes.

UPDATE

It might be easier so use the indexOf method to find the team name in the List and remove the required elements

public void remove(final List<String> teams, final String name) {
    final int idx = teams.indexOf(name);
    if(idx < 0) 
        throw new IllegalArgumentException("Team " + name + " not present in list.");
    for(int i = idx + 2; i >= idx; --i) 
        teams.remove(i);
}

It is very important to remove the items in reverse. This is due to the same problem as above, if you remove the item at index (for example) 10 then the item that was at index 11 is moved down. So when you go to remove the item at index 11 you are actually removing the item what initially was it index 12.

You can use a dirty trick

for(int i = 0; i < 2; ++i)
    teams.remove(idx) 

I.e. keep removing the item at the found index, as the list will shuffle downwards to fill the gap you will actually remove the required item and the two that were above it. I think this makes code very hard to read. You will probably forget this trick and then come back to the code and have to work out what it is doing.