Collection problem

[OffbeatPatriot]

I have a class, Cast(as in cast of a play), that implements the Collection<Actor> interface(Actor is a class representing an entity in a game) and I have the following code.

Set<Actor> toRemove = new HashSet(locations.keySet());
//method 1
toRemove.removeAll(actors);
//method 2
for(Actor actor : actors){
    toRemove.remove(actor);
}

if I use method 1 nothing gets removed from the toRemove set, but method 2 does just what I want. I'm doing fine with method 2 but I'm curious why 1 doesn't work. This is the Cast class.

package Felidae.Actors;

import Felidae.Physics.*;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.List;
import java.util.LinkedList;

public class Cast implements Collection<Actor> {
    private List<Actor> actorList;
    private Map<String, Actor> actorMap;

    public Cast(){
        actorList = new LinkedList<Actor>();
        actorMap = new HashMap<String, Actor>();
    }

    public int size() {
        return actorList.size();
    }

    public boolean isEmpty() {
        return actorList.isEmpty();
    }

    public boolean contains(Object o) {
        if(o instanceof Actor){
            return actorList.contains((Actor)o);
        }else{
            return false;
        }
    }

    public Iterator iterator() {
        return actorList.iterator();
    }

    public Object[] toArray() {
        return actorList.toArray();
    }

    public Object[] toArray(Object[] a) {
        return actorList.toArray(a);
    }

    public boolean add(Actor o) {
        actorMap.put(o.name, o);
        return actorList.add(o);
    }

    public boolean remove(Object o) {
        if(o instanceof Actor){
            Actor a = (Actor)o;
            if(!a.name.isEmpty()){
                actorMap.remove(a.name);
            }
            return actorList.remove(a);
        }else{
            return false;
        }
    }

    public boolean containsAll(Collection c) {
        return actorList.containsAll(c);
    }

    public boolean addAll(Collection c) {
        boolean result = actorList.addAll(c);
        for(Object o : c){
            if(o instanceof Actor){
                Actor a = (Actor)o;
                if(!a.name.isEmpty()){
                    actorMap.put(a.name, a);
                }
            }
        }
        return result;
    }

    public boolean removeAll(Collection c) {
        boolean result = actorList.removeAll(c);
        for(Object o : c){
            if(o instanceof Actor){
                Actor a = (Actor)o;
                if(!a.name.isEmpty()){
                    actorMap.remove(a.name);
                }
            }
        }
        return result;
    }

    public boolean retainAll(Collection c) {
        return actorList.retainAll(c);
    }

    public void clear() {
        actorList.clear();
    }

    public Actor get(String name){
        return actorMap.get(name);
    }
}

[javaAddict]

Because the toRemove, doesn't have an Element actors:

toRemove.removeAll(actors);

Look the API, it removes the element you put as input. I assumed that added Actor instances, not Collection instances.

Also look at the API for a better description

[OffbeatPatriot]

Something I didn't mention, actors is an instance of Cast, and location is a map with Actors for keys.

I'm looking at the documentation for removeAll and it takes a collection, and removes all elements in that collection.

[javaAddict]

I don't know if this will fix your problem but have you tried to declare the method like this:

boolean removeAll(Collection<Actor> c)

Also in order for the "removing" to take place, java compares the elements in your list with the ones in the Collection in order to remove them. I order to compare them it checks if they are equal with each other. If yes it removes that element. So it uses the "equals" method. If you haven't overridden that method in the Actor class then the "equals" of the super class Object will be used which checks if they are the same instance, not if they have the same values.

Have you overridden the equals method in the Actor class?

[BestJewSinceJC]

JavaAddict: Good suggestions but for the one suggestion, it shouldn't matter if he implemented equals() since he is using the removeAll method. Right?

OP: If none of JavaAddicts solutions work, please post all of your code and I will run it and play with it and help you figure out what is wrong. JavaAddict is giving you good help, but sometimes there is only so much you can do by looking at the code when the code is complex.

[javaAddict]

JavaAddict: Good suggestions but for the one suggestion, it shouldn't matter if he implemented equals() since he is using the removeAll method. Right?

He is not using a removeAll method with no arguments. Perhaps you didn't notice but that method takes as argument a collection and removes those elements that are inside the collection.

[OffbeatPatriot]

If I don't implement equals is it equivalent to the == expression? Because that's what I wanted.

[BestJewSinceJC]

He is not using a removeAll method with no arguments. Perhaps you didn't notice but that method takes as argument a collection and removes those elements that are inside the collection.

Yeah, I was confused because I'm using to seeing the no-argument version, even though I looked at his code. I got it now after reading the API, sorry for the misunderstanding.

@ OP

You wanted to use ==? You realize that will compare memory addresses or references? Oh, and if you don't implement the equals method, the method will be inherited from Object (or somewhere along the hierarchy), so you may not get the behavior you want anyway. You could research this by looking at the code and seeing how it is implemented.

[OffbeatPatriot]

Here's the bigger picture:

This code is for the physics of a game, and this code is in a class called Partition that gets a list of actors on construction which it keeps a reference to(actors). These actors represent each entity in the game and initially the Partition divides the level into a grid and the Partition makes a two dimensional ArrayList of Sets of actors where each set represents the actors in a particular cell of the grid. Locations is a HashMap<Actor, Set<Integer>> that maps each actor to every cell they occupy.

As the game runs the partition has to keep it's representation of the grid current, so actors have to be moved, and removed. The code I posted is the first step in removing actors and reads something like this, "If an actor is listed in Locations as occupying space on the grid, and yet the central collection of actors doesn't contain it, it is no longer part of the game and must be removed from the grid."

So toRemove should contain references to instances of Actor which are in locations.keySet() yet not in actors.