Improve sector-related code, add SectorOperation so that SectorListeners can filter by operation type.

src/org/apollo/game/model/area/Sector.java

@@ -11,6 +11,8 @@ import org.apollo.game.model.Position;
 import org.apollo.game.model.entity.Entity;
 import org.apollo.game.model.entity.Entity.EntityType;
 
+import com.google.common.collect.ImmutableList;
+
 /**
  * An 8x8 area of the map.
  * 
@@ -19,7 +21,7 @@ import org.apollo.game.model.entity.Entity.EntityType;
 public final class Sector {
 
 	/**
-	 * The width and height of a sector, in tiles.
+	 * The width and length of a sector, in tiles.
 	 */
 	public static final int SECTOR_SIZE = 8;
 
@@ -72,7 +74,7 @@ public final class Sector {
 
 		entities.add(entity);
 		this.entities.put(position, entities);
-		notifyListeners(entity);
+		notifyListeners(entity, SectorOperation.ADD);
 	}
 
 	/**
@@ -98,7 +100,7 @@ public final class Sector {
 	}
 
 	/**
-	 * Gets a copy of the {@link List} of {@link Entity}s.
+	 * Gets a shallow copy of the {@link List} of {@link Entity}s. The returned type will be {@link ImmutableList}.
 	 * 
 	 * @param position The position containing the entities.
 	 * @return The list.
@@ -110,34 +112,38 @@ public final class Sector {
 			return Collections.emptyList();
 		}
 
-		return new ArrayList<>(entities);
+		return ImmutableList.copyOf(entities);
 	}
 
 	/**
-	 * Gets a copy of the {@link List} of {@link Entity}s with the specified {@link EntityType}.
+	 * Gets a shallow copy of the {@link List} of {@link Entity}s with the specified {@link EntityType}. The returned
+	 * list will be an {@link ImmutableList}. Type will be inferred from the call, so ensure that the entity type and
+	 * the reference correspond, or this method will fail at runtime.
 	 * 
 	 * @param position The {@link Position} containing the entities.
 	 * @param type The {@link EntityType}.
 	 * @return The list of entities.
 	 */
-	@SuppressWarnings("unchecked")
 	public <T extends Entity> List<T> getEntities(Position position, EntityType type) {
 		List<Entity> entities = this.entities.get(position);
 		if (entities == null) {
 			this.entities.put(position, new ArrayList<>());
-			return Collections.emptyList();
+			return ImmutableList.of();
 		}
 
-		return (List<T>) entities.stream().filter(e -> e.getEntityType() == type).collect(Collectors.toList());
+		@SuppressWarnings("unchecked")
+		List<T> filtered = (List<T>) entities.stream().filter(e -> e.getEntityType() == type).collect(Collectors.toList());
+		return ImmutableList.copyOf(filtered);
 	}
 
 	/**
-	 * Notifies the listeners registered to this sector that an update has occurred.
+	 * Notifies the {@link SectorListener}s registered to this sector that an update has occurred.
 	 * 
-	 * @param entity The entity that was updated.
+	 * @param entity The {@link Entity} that was updated.
+	 * @param operation The {@link SectorOperation} that occurred.
 	 */
-	public void notifyListeners(Entity entity) {
+	public void notifyListeners(Entity entity, SectorOperation operation) {
-		listeners.forEach(l -> l.execute(this, entity));
+		listeners.forEach(listener -> listener.execute(this, entity, operation));
 	}
 
 	/**
@@ -149,13 +155,9 @@ public final class Sector {
 	public boolean removeEntity(Entity entity) {
 		Position position = entity.getPosition();
 		List<Entity> entities = this.entities.get(position);
-		if (entities == null) {
-			this.entities.put(position, new ArrayList<>());
-			return false;
-		}
 		
-		if (entities.remove(entity)) {
+		if (entities != null && entities.remove(entity)) {
-			notifyListeners(entity);
+			notifyListeners(entity, SectorOperation.REMOVE);
 			return true;
 		}
 		return false;

src/org/apollo/game/model/area/SectorListener.java

@@ -3,7 +3,8 @@ package org.apollo.game.model.area;
 import org.apollo.game.model.entity.Entity;
 
 /**
- * A class that should be extended by listeners that execute actions when an entity is added or removed from the sector.
+ * A class that should be implemented by listeners that execute actions when an entity is added or removed from the
+ * sector.
  * 
  * @author Major
  */
@@ -13,9 +14,10 @@ public interface SectorListener {
 	/**
 	 * Executes the action for this listener.
 	 * 
-	 * @param sector The sector that was updated.
+	 * @param sector The {@link Sector} that was updated.
-	 * @param entity The affected entity.
+	 * @param entity The affected {@link Entity}.
+	 * @param operation The type of {@link SectorOperation}.
 	 */
-	public abstract void execute(Sector sector, Entity entity);
+	public abstract void execute(Sector sector, Entity entity, SectorOperation operation);
 
 }

src/org/apollo/game/model/area/SectorOperation.java

@@ -0,0 +1,20 @@
+package org.apollo.game.model.area;
+
+/**
+ * An operation that can be performed by a sector, used by {@link SectorListener}s to differentiate between operations.
+ * 
+ * @author Major
+ */
+public enum SectorOperation {
+
+	/**
+	 * The add operation, when an entity has been added to a sector.
+	 */
+	ADD,
+
+	/**
+	 * The remove operation, when an entity has been removed from a sector.
+	 */
+	REMOVE;
+
+}

src/org/apollo/game/model/area/SectorRepository.java

@@ -1,15 +1,17 @@
 package org.apollo.game.model.area;
 
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 import org.apollo.game.model.Position;
 
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+
 /**
- * A repository of {@link Sector Sectors}, backed by a {@link HashMap} of {@link SectorCoordinates} that correspond to
+ * A repository of {@link Sector}s, backed by a {@link HashMap} of {@link SectorCoordinates} that correspond to their
- * their appropriate sectors.
+ * appropriate sectors.
  * 
  * @author Major
  */
@@ -28,7 +30,7 @@ public final class SectorRepository {
 	/**
 	 * Creates a new sector repository.
 	 * 
-	 * @param permitRemoval If removal (of {@link Sector Sectors}) from this repository should be permitted.
+	 * @param permitRemoval If removal (of {@link Sector}s) from this repository should be permitted.
 	 */
 	public SectorRepository(boolean permitRemoval) {
 		this.permitRemoval = permitRemoval;
@@ -43,11 +45,9 @@ public final class SectorRepository {
 	 *             existing sector would be replaced), and removal of sectors is not permitted.
 	 */
 	public void add(Sector sector) {
-		if (sector == null) {
+		Preconditions.checkNotNull(sector, "Sector cannot be null.");
-			throw new IllegalArgumentException("Sector cannot be null.");
+		if (sectors.containsKey(sector.getCoordinates()) && !permitRemoval) {
-		} else if (sectors.containsKey(sector.getCoordinates()) && !permitRemoval) {
+			throw new UnsupportedOperationException("Cannot add a sector with the same coordinates as an existing sector.");
-			throw new UnsupportedOperationException(
-					"Cannot add a sector with the same coordinates as an existing sector.");
 		}
 		sectors.put(sector.getCoordinates(), sector);
 	}
@@ -56,10 +56,11 @@ public final class SectorRepository {
 	 * Indicates whether the supplied value (i.e. the {@link Sector}) has a mapping.
 	 * 
 	 * @param sector The sector.
-	 * @return {@code true} if the value is mapped by a key (i.e. {@link SectorCoordinates}), otherwise {@code false}.
+	 * @return {@code true} if this repository contains an entry with {@link SectorCoordinates} equal to the specified
+	 *         sector, otherwise {@code false}.
 	 */
 	public boolean contains(Sector sector) {
-		return sectors.containsValue(sector);
+		return contains(sector.getCoordinates());
 	}
 
 	/**
@@ -88,7 +89,7 @@ public final class SectorRepository {
 	 * repository then a new sector is created, submitted to the repository, and returned.
 	 * 
 	 * @param coordinates The coordinates.
-	 * @return The sector.
+	 * @return The sector. Will never be null.
 	 */
 	public Sector get(SectorCoordinates coordinates) {
 		Sector sector = sectors.get(coordinates);
@@ -100,26 +101,27 @@ public final class SectorRepository {
 	}
 
 	/**
-	 * Gets the {@link List} of {@link Sector Sectors}. This is a shallow copy (i.e. modifying the list will not change
+	 * Gets a shallow copy of the {@link List} of {@link Sector}s. This will be an {@link ImmutableList}.
-	 * the repository, but modifying the sectors in the list will change the sectors in this repository).
 	 * 
 	 * @return The list.
 	 */
 	public List<Sector> getSectors() {
-		return new ArrayList<>(sectors.values());
+		return ImmutableList.copyOf(sectors.values());
 	}
 
 	/**
-	 * Removes a {@link Sector} from the repository, if permitted.
+	 * Removes a {@link Sector} from the repository, if permitted. This method removes the entry that has a key
+	 * identical to the {@link SectorCoordinates} of the specified sector.
 	 * 
 	 * @param sector The sector to remove.
+	 * @return Whether or not the sector was removed.
 	 * @throws UnsupportedOperationException If this method is called on a repository that does not permit removal.
 	 */
-	public void remove(Sector sector) {
+	public boolean remove(Sector sector) {
 		if (!permitRemoval) {
 			throw new UnsupportedOperationException("Cannot remove sectors from this repository.");
 		}
-		sectors.remove(sector.getCoordinates());
+		return sectors.remove(sector.getCoordinates()) != null;
 	}
 
 }