More placement code cleanup. (#11823)

No functional changes. - Simplify checks for unit placement restrictions with a helper. - Use HashSet for temporary collections that are used for contains() checks.
triplea-game · Jul 26, 2023 · a51a18b · a51a18b
1 parent 2aa26ae
commit a51a18b
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 28 deletions.
diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/attachments/UnitAttachment.java b/game-app/game-core/src/main/java/games/strategy/triplea/attachments/UnitAttachment.java
@@ -908,6 +908,13 @@ public String[] getUnitPlacementRestrictions() {
     return unitPlacementRestrictions;
   }
 
+  public boolean unitPlacementRestrictionsContain(Territory territory) {
+    if (unitPlacementRestrictions == null) {
+      return false;
+    }
+    return Arrays.asList(unitPlacementRestrictions).contains(territory.getName());
+  }
+
   private void resetUnitPlacementRestrictions() {
     unitPlacementRestrictions = null;
   }

diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/AbstractPlaceDelegate.java b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/AbstractPlaceDelegate.java
@@ -28,6 +28,7 @@
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -459,7 +460,7 @@ private void freePlacementCapacity(
     boolean unusedSplitPlacements = false;
     if (foundSpaceTotal < freeSize) {
       // we need to split some placement moves
-      final Collection<UndoablePlacement> usedUndoablePlacements = new ArrayList<>();
+      final var usedUndoablePlacements = new HashSet<UndoablePlacement>();
       for (final Tuple<UndoablePlacement, Territory> tuple : splitPlacements) {
         final UndoablePlacement placement = tuple.getFirst();
         if (usedUndoablePlacements.contains(placement)) {
@@ -880,9 +881,7 @@ public String canUnitsBePlaced(
             + to.getName()
             + " due to Unit Placement Restrictions on Territory Value";
       }
-      final String[] terrs = ua.getUnitPlacementRestrictions();
-      final Collection<Territory> listedTerrs = getListedTerritories(terrs);
-      if (listedTerrs.contains(to)) {
+      if (ua.unitPlacementRestrictionsContain(to)) {
         return "Cannot place these units in "
             + to.getName()
             + " due to Unit Placement Restrictions";
@@ -945,7 +944,7 @@ protected Collection<Unit> getUnitsToBePlaced(
     if (units.stream().anyMatch(Matches.unitIsConstruction())) {
       final IntegerMap<String> constructionsMap =
           howManyOfEachConstructionCanPlace(to, to, units, player);
-      final Collection<Unit> skipUnits = new ArrayList<>();
+      final var skipUnits = new HashSet<Unit>();
       for (final Unit currentUnit :
           CollectionUtils.getMatches(units, Matches.unitIsConstruction())) {
         final int maxUnits = howManyOfConstructionUnit(currentUnit, constructionsMap);
@@ -979,7 +978,7 @@ protected Collection<Unit> getUnitsToBePlaced(
     }
     // now check stacking limits
     final Collection<Unit> placeableUnits2 = new ArrayList<>();
-    final Collection<UnitType> typesAlreadyChecked = new ArrayList<>();
+    final var typesAlreadyChecked = new HashSet<UnitType>();
     for (final Unit currentUnit : placeableUnits) {
       final UnitType ut = currentUnit.getType();
       if (typesAlreadyChecked.contains(ut)) {
@@ -1013,9 +1012,7 @@ protected Collection<Unit> getUnitsToBePlaced(
         continue;
       }
       // account for any unit placement restrictions by territory
-      final String[] terrs = ua.getUnitPlacementRestrictions();
-      final Collection<Territory> listedTerrs = getListedTerritories(terrs);
-      if (!listedTerrs.contains(to)) {
+      if (!ua.unitPlacementRestrictionsContain(to)) {
         placeableUnits3.add(currentUnit);
       }
     }
@@ -1367,9 +1364,7 @@ IntegerMap<String> howManyOfEachConstructionCanPlace(
       final UnitAttachment ua = currentUnit.getUnitAttachment();
       // account for any unit placement restrictions by territory
       if (Properties.getUnitPlacementRestrictions(getData().getProperties())) {
-        final String[] terrs = ua.getUnitPlacementRestrictions();
-        final Collection<Territory> listedTerrs = getListedTerritories(terrs);
-        if (listedTerrs.contains(to)) {
+        if (ua.unitPlacementRestrictionsContain(to)) {
           continue;
         }
         if (ua.getCanOnlyBePlacedInTerritoryValuedAtX() != -1
@@ -1744,22 +1739,6 @@ private static boolean isPlacementInCapitalRestricted(final GamePlayer player) {
     return ra != null && ra.getPlacementInCapitalRestricted();
   }
 
-  private Collection<Territory> getListedTerritories(final String[] list) {
-    final List<Territory> territories = new ArrayList<>();
-    if (list == null) {
-      return territories;
-    }
-    for (final String name : list) {
-      // Validate all territories exist
-      final Territory territory = getData().getMap().getTerritory(name);
-      if (territory == null) {
-        throw new IllegalStateException("Rules & Conditions: No territory called:" + name);
-      }
-      territories.add(territory);
-    }
-    return territories;
-  }
-
   @Override
   public Class<? extends IRemote> getRemoteType() {
     return IAbstractPlaceDelegate.class;