Skip to content

Commit

Permalink
Remove unnecessary null checks and consolidate ChangeFactory methods. (
Browse files Browse the repository at this point in the history
…#10531)

* Remove unnecessary null checks and consolidate ChangeFactory methods.

The null checks are no longer needed due to changes to attachment collection getters, that always return non-null.

* Also remove checks for null UnitAttachment, which is always present.

* Remove a few more null checks.

* Remove more null checks.
  • Loading branch information
asvitkine authored May 30, 2022
1 parent deb89b0 commit a5577ad
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import games.strategy.engine.data.TechnologyFrontier;
import games.strategy.engine.data.Territory;
import games.strategy.engine.data.Unit;
import games.strategy.engine.data.UnitHolder;
import games.strategy.engine.data.changefactory.units.BombingUnitDamageChange;
import games.strategy.engine.data.changefactory.units.UnitDamageReceivedChange;
import games.strategy.triplea.attachments.TechAttachment;
Expand Down Expand Up @@ -71,20 +72,12 @@ public static Change changeOwner(
return new PlayerOwnerChange(units, owner, location);
}

public static Change addUnits(final Territory territory, final Collection<Unit> units) {
return new AddUnits(territory.getUnitCollection(), units);
public static Change addUnits(final UnitHolder holder, final Collection<Unit> units) {
return new AddUnits(holder.getUnitCollection(), units);
}

public static Change addUnits(final GamePlayer player, final Collection<Unit> units) {
return new AddUnits(player.getUnitCollection(), units);
}

public static Change removeUnits(final Territory territory, final Collection<Unit> units) {
return new RemoveUnits(territory.getUnitCollection(), units);
}

public static Change removeUnits(final GamePlayer player, final Collection<Unit> units) {
return new RemoveUnits(player.getUnitCollection(), units);
public static Change removeUnits(final UnitHolder holder, final Collection<Unit> units) {
return new RemoveUnits(holder.getUnitCollection(), units);
}

public static Change moveUnits(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public static Predicate<Unit> unitIsTransportAndNotDestroyer() {
public static Predicate<UnitType> unitTypeIsStrategicBomber() {
return obj -> {
final UnitAttachment ua = UnitAttachment.get(obj);
return ua != null && ua.getIsStrategicBomber();
return ua.getIsStrategicBomber();
};
}

Expand Down Expand Up @@ -283,18 +283,12 @@ public static Predicate<Unit> unitCanBeCapturedOnEnteringToInThisTerritory(
if (ta == null) {
return false;
}
if (ta.getCaptureUnitOnEnteringBy() == null) {
return false;
}
final boolean territoryCanHaveUnitsThatCanBeCapturedByPlayer =
ta.getCaptureUnitOnEnteringBy().contains(player);
final PlayerAttachment pa = PlayerAttachment.get(unitOwner);
if (pa == null) {
return false;
}
if (pa.getCaptureUnitOnEnteringBy() == null) {
return false;
}
final boolean unitOwnerCanLetUnitsBeCapturedByPlayer =
pa.getCaptureUnitOnEnteringBy().contains(player);
return (unitCanBeCapturedByPlayer
Expand Down Expand Up @@ -1512,7 +1506,7 @@ static Predicate<Unit> unitCanRepairOthers() {
return false;
}
final UnitAttachment ua = UnitAttachment.get(unit.getType());
return ua.getRepairsUnits() != null && !ua.getRepairsUnits().isEmpty();
return !ua.getRepairsUnits().isEmpty();
};
}

Expand Down Expand Up @@ -1548,8 +1542,7 @@ static Predicate<Unit> unitCanRepairThisUnit(
}
}
final UnitAttachment ua = UnitAttachment.get(unitCanRepair.getType());
return ua.getRepairsUnits() != null
&& ua.getRepairsUnits().keySet().contains(damagedUnit.getType());
return ua.getRepairsUnits().keySet().contains(damagedUnit.getType());
};
}

Expand Down Expand Up @@ -1655,30 +1648,28 @@ public static Predicate<Unit> unitCanBeGivenBonusMovementByFacilitiesInItsTerrit
static Predicate<Unit> unitCreatesUnits() {
return unit -> {
final UnitAttachment ua = UnitAttachment.get(unit.getType());
return ua != null && ua.getCreatesUnitsList() != null && !ua.getCreatesUnitsList().isEmpty();
return !ua.getCreatesUnitsList().isEmpty();
};
}

static Predicate<Unit> unitCreatesResources() {
return unit -> {
final UnitAttachment ua = UnitAttachment.get(unit.getType());
return ua != null
&& ua.getCreatesResourcesList() != null
&& !ua.getCreatesResourcesList().isEmpty();
return !ua.getCreatesResourcesList().isEmpty();
};
}

public static Predicate<UnitType> unitTypeConsumesUnitsOnCreation() {
return unit -> {
final UnitAttachment ua = UnitAttachment.get(unit);
return ua != null && !ua.getConsumesUnits().isEmpty();
return !ua.getConsumesUnits().isEmpty();
};
}

public static Predicate<Unit> unitConsumesUnitsOnCreation() {
return unit -> {
final UnitAttachment ua = UnitAttachment.get(unit.getType());
return ua != null && !ua.getConsumesUnits().isEmpty();
return !ua.getConsumesUnits().isEmpty();
};
}

Expand Down Expand Up @@ -1720,7 +1711,7 @@ public static Predicate<Unit> eligibleUnitToConsume(GamePlayer owner, UnitType u
public static Predicate<Unit> unitRequiresUnitsOnCreation() {
return unit -> {
final UnitAttachment ua = UnitAttachment.get(unit.getType());
return ua != null && ua.getRequiresUnits() != null && !ua.getRequiresUnits().isEmpty();
return !ua.getRequiresUnits().isEmpty();
};
}

Expand Down Expand Up @@ -1769,9 +1760,7 @@ public static Predicate<Unit> unitWhichRequiresUnitsHasRequiredUnitsInList(
public static Predicate<Unit> unitHasRequiredUnitsToMove(final Territory t) {
return unit -> {
final UnitAttachment ua = UnitAttachment.get(unit.getType());
if (ua == null
|| ua.getRequiresUnitsToMove() == null
|| ua.getRequiresUnitsToMove().isEmpty()) {
if (ua.getRequiresUnitsToMove().isEmpty()) {
return true;
}

Expand Down Expand Up @@ -1810,7 +1799,7 @@ static Predicate<Territory> territoryIsBlockadeZone() {
public static Predicate<UnitType> unitTypeIsConstruction() {
return type -> {
final UnitAttachment ua = UnitAttachment.get(type);
return ua != null && ua.getIsConstruction();
return ua.getIsConstruction();
};
}

Expand Down Expand Up @@ -1998,7 +1987,7 @@ public static Predicate<Unit> unitHasWhenCombatDamagedEffect(final String filter
public static Predicate<Territory> territoryHasCaptureOwnershipChanges() {
return t -> {
final TerritoryAttachment ta = TerritoryAttachment.get(t);
return (ta != null) && !ta.getCaptureOwnershipChanges().isEmpty();
return ta != null && !ta.getCaptureOwnershipChanges().isEmpty();
};
}

Expand Down Expand Up @@ -2129,7 +2118,7 @@ public static Predicate<Territory> airCanLandOnThisAlliedNonConqueredLandTerrito
return false;
}
final GamePlayer owner = t.getOwner();
if (owner == null || owner.isNull()) {
if (owner.isNull()) {
return false;
}
final RelationshipTracker rt = data.getRelationshipTracker();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ private static IntegerMap<UnitType> getCostsForTuvForAllPlayersMergedAndAveraged
// Add any units that have XML TUV even if they aren't purchasable
for (final UnitType unitType : data.getUnitTypeList()) {
final UnitAttachment ua = UnitAttachment.get(unitType);
if (ua != null && ua.getTuv() > 0) {
if (ua.getTuv() > 0) {
costs.put(unitType, ua.getTuv());
}
}
Expand All @@ -149,11 +149,11 @@ private static IntegerMap<UnitType> getCostsForTuvForAllPlayersMergedAndAveraged
private static int getTotalTuv(
final UnitType unitType, final IntegerMap<UnitType> costs, final Set<UnitType> alreadyAdded) {
final UnitAttachment ua = UnitAttachment.get(unitType);
if (ua != null && ua.getTuv() > 0) {
if (ua.getTuv() > 0) {
return ua.getTuv();
}
int tuv = costs.getInt(unitType);
if (ua == null || ua.getConsumesUnits().isEmpty() || alreadyAdded.contains(unitType)) {
if (ua.getConsumesUnits().isEmpty() || alreadyAdded.contains(unitType)) {
return tuv;
}
alreadyAdded.add(unitType);
Expand Down

0 comments on commit a5577ad

Please sign in to comment.