Increment object properties based on a particular subclass of an abstract class

Question

I have a Composition object that contains 4 ArrayList objects, each of which contains objects of a particular subclass of my Abstract class Role. The 4 subclasses are Bruiser, Healer, Tank, and DamageDealer. The point of the program is to analyze two opposing Composition objects as they are being built (players fill their Composition object one Role pick at a time), and based on each of their properties, recommend to the player a particular Hero for a particular Role that would suit the player's Composition.

The values of the Composition object's properties are incremented accordingly each time a Role subclass gets added to one of its ArrayLists, like so (note: "this" corresponds to the Composition object):

public class Composition {
 private ArrayList<Tank> tanks;
 private ArrayList<DamageDealer> damageDealers;
 private ArrayList<Healer> healers;
 private ArrayList<Bruiser> bruisers;
 private boolean doubleSupport;
 private boolean doubleWarrior;
 private boolean heavyDamage;
 private boolean lockdownProtection;
 private int mobility;
 private int waveclear;
 private int nuMelee;
 private int nuRange;
 private int lockdown;
 private int burstMitigation;
 private int sustainedMitigation;
 private int damage;
 private int burstDamageRating;
 private int sustainedDamageRating;
 private int frontlineRating;
 private int defensiveRating;
 private int offensiveRating;
 public Composition() {
 this.tanks = new ArrayList<Tank>();
 this.damageDealers = new ArrayList<DamageDealer>();
 this.healers = new ArrayList<Healer>();
 this.bruisers = new ArrayList<Bruiser>();
 }
 private void addMutualContribution(Role r) {
 this.mobility += r.getMobility();
 this.defensiveRating += r.getPeeling();
 this.waveclear += r.getWaveclear();
 this.burstMitigation += r.getBurstMitigaion();
 this.sustainedMitigation += r.getSustainedMitigation();
 }
 public void addBruiserContribution(Bruiser b) {
 addMutualContribution(b);
 this.offensiveRating += b.getPressure();
 this.damage += b.getDamage();
 this.frontlineRating += b.getFrontlineRating();
 addBruiser(b);
 }
 public void addDDContribution(DamageDealer d) {
 addMutualContribution(d);
 this.burstDamageRating += d.getBurstDamageRating();
 this.sustainedDamageRating += d.getSustainedDamageRating();
 this.damage += d.getDamageOutput();
 this.offensiveRating += d.getAggression();
 addDamageDealer(d);
 }
 public void addTankContribution(Tank t) {
 addMutualContribution(t);
 this.defensiveRating += t.getAntiMeleeEffectiveness();
 this.offensiveRating += t.getEngage();
 this.frontlineRating += t.getFrontlineRating();
 addTank(t);
 }
 public void addHealerContribution(Healer h) {
 addMutualContribution(h);
 this.burstMitigation += h.getBurstHealing();
 this.sustainedMitigation += h.getSustainedHealing();
 if (h.hasLockdownProtection()) {
 this.lockdownProtection = true;
 }
 addHealer(h);
 }
 private void addTank(Tank t) {
 tanks.add(t);
 if (tanks.size() + bruisers.size() > 1) {
 setDoubleWarrior(true);
 }
 }
 private void addHealer(Healer h) {
 healers.add(h);
 if (healers.size() == 2) {
 setDoubleSupport(true);
 }
 }
 private void addDamageDealer(DamageDealer d) {
 damageDealers.add(d);
 if (damageDealers.size() + bruisers.size() > 2) {
 setHeavyDamage(true);
 }
 }
 private void addBruiser(Bruiser b) {
 bruisers.add(b);
 if(bruisers.size() + damageDealers.size() > 2) {
 setHeavyDamage(true);
 }
 }
 private void setDoubleWarrior(boolean b) {
 doubleWarrior = b;
 }
 private void setDoubleSupport(boolean b) {
 doubleSupport = b;
 }
 private void setHeavyDamage(boolean b) {
 heavyDamage = b;
 }
}

To me, this implementation feels backwards. Ultimately I should only have the one public method to add a Role subclass to the composition, then the Composition should privately adjust its own properties based on which particular subclass is being added, something like this:

public void addRole(CustomEnum roleType, Role r) {
 // increment properties inherent to all roles like mobility, waveclear, etc.
 // ...
 switch (roleType) {
 case BRUISER:
 addBruiserContribution(r);
 break;
 case TANK:
 addTankContribution(r);
 break;
 case DAMAGE_DEALER:
 addDDContribution(r);
 break;
 case HEALER:
 addHealerContribution(r);
 break;
 default:
 throw new InvalidArgumentException("Wrong roletype provided!");
 }
}

This doesn't work without me having to hardcast the Role abstract class into a particular subclass before passing it to the appropriate add__Contribution() method, which is a big red-flag. Thus it feels like I missed an opportunity for cleaner polymorphism, but I can't figure out where.

How could I make my code well-formed and polymorphic to suit my needs?

Role Abstract class:

public abstract class Role {
 protected String name;
 protected int mobility;
 protected int waveclear;
 protected int peeling;
 protected int burstMitigation;
 protected int sustainedMitigation;
 protected boolean lockdown;
 protected boolean aaControl;
 protected AttackRange range;
 public String getName() {
 return name;
 }
 public int getMobility() {
 return mobility;
 }
 public int getWaveclear() {
 return waveclear;
 }
 public int getPeeling() {
 return peeling;
 }
 public int getBurstMitigaion() {
 return burstMitigation;
 }
 public int getSustainedMitigation() {
 return sustainedMitigation;
 }
 public boolean hasLockdown() {
 return lockdown;
 }
 public boolean hasAAControl() { 
 return aaControl;
 }
 public AttackRange getRange() {
 return range;
 }
}

Bruiser Subclass:

public class Bruiser extends Role {
 private int pressure;
 private int damage;
 private int frontlineRating;
 protected Bruiser() {
 }
 public int getPressure() {
 return pressure;
 }
 public int getDamage() {
 return damage;
 }
 public int getFrontlineRating() {
 return frontlineRating;
 }
}

Healer subclass:

public class Healer extends Role {
 private int burstHealing;
 private int sustainedHealing;
 private boolean lockdownProtection;
 protected Healer() {
 }
 public int getBurstHealing() {
 return burstHealing;
 }
 public int getSustainedHealing() {
 return sustainedHealing;
 }
 public boolean hasLockdownProtection() {
 return lockdownProtection;
 }
}

Tank subclass:

public class Tank extends Role {
 private int antiMeleeEffectiveness; 
 private int frontlineRating;
 private int engage;
 protected Tank() {
 }
 public int getAntiMeleeEffectiveness() {
 return antiMeleeEffectiveness;
 }
 public int getFrontlineRating() {
 return frontlineRating;
 }
 public int getEngage() {
 return engage;
 }
}

Damage Dealer subclass:

public class DamageDealer extends Role {
 private int burstDamageRating;
 private int sustainedDamageRating;
 private int aggression;
 private int damageOutput;
 protected DamageDealer() {
 }
 public int getBurstDamageRating() {
 return burstDamageRating;
 }
 public int getSustainedDamageRating() {
 return sustainedDamageRating;
 }
 public int getAggression() {
 return aggression;
 }
 public int getDamageOutput() {
 return damageOutput;
 }
}

Composition Class (minus property "getters"):

• t3chb0t: How could I make my code well-formed and polymorphic to suit my needs? I suggest posting your complete and real code. We cannot give you advice for code that isn't there.
• David Tamrazov: @t3chb0t noted! I've edited my question to include all of the code relevant to the classes I'm trying to refactor.
• ChrisWue: It's difficult to say what exact role the composition class plays. I'm going to guess it's some sort of "battle group" to which the individual units contribute and the overall battle group properties are based on the individual units within. So one option is to add a updateGroupContribution(Group groupToContributeTo) method to the Role class. Then each role can update the group it's being added to by adding to its properties. Adding a role to the composite class is then as easy as addContribution(Role r) { r.updateGroupContribution(this); }
• ChrisWue: btw: Adding the code for the composite would be useful since that's where some of the problems stem from
• David Tamrazov: @ChrisWue I've added all of the code for the "Composition" class minus the property getters, and included a brief description of the program in my body in a new edit.

Answer 1

Commenting on the snippets you published and your question:

Thus it feels like I missed an opportunity for cleaner polymorphism, but I can't figure out where.

The problem I see is in the mapping from Role subclass properties that aren't present in all Roles to Composition properties.

An example is this.offensiveRating += b.getPressure();. I'd recommend to include an abstract getPressure() into the Role abstract class (maybe renamed to getOffensiveRating() to better match its intent). For all subclasses that don't have a "pressure" implement the method returning 0.

Then you don't need any case distinctions, just one addContribution(Role r) will do the job.

An alternative might be to have a non-abstract Role.getPressure() method returning 0, to be overridden only by Roles that hava a pressure.

• David Tamrazov: that's a solid solution! I like the idea of matching Role properties to those of Composition since thats the most important place the Role subclasses contribute to my program. I'm gonna leave my question up for another day just to see what other suggestions are out there, now that I edited it and fleshed out the code some more, but I'm a big fan of this option. Thank you!

Answer 2

I think the main cause for the complications you mention is the fact that the state of a Composition object can, in essence, be reduced to the Lists of the four subtypes of Role (by the way, unless you have a specific reason for the Lists not to be any implementation of List but indeed an ArrayList, you could declare these fields as Lists instead of ArrayLists) and all other fields depend solely on these four lists. So you have the same property stored in muliple places, which naturally makes things more complicated, because you have to ensure that the different representations of the properties (i.e. the Role lists on the one hand, and the individual fields for each property on the other hand) don't contradict each other.

A way to circumvent this would be discard the fields storing the individual properties and replace them with methods that calculate a property based on the four Role lists, like this:

public boolean hasDoubleSupport() {
 return healers.size() >= 2;
}
public int mobility() {
 return Stream.of(
 tanks.stream(),
 damageDealers.stream(),
 healers.stream(),
 bruisers.stream()) // inferred type T for Stream.<T>of(T...) will be Stream<? extends Role>
 .flatMapToInt(stream -> stream.mapToInt(Role::getMobility))
 .sum();
}
public int offensiveRating() {
 return tanks.stream().mapToInt(Tank::getEngage).sum()
 + damageDealers.stream().mapToInt(DamageDealer::getAggression).sum()
 + bruisers.stream().mapToInt(Bruiser::getPressure).sum();
}

Also, I don't know if the properties of a specific Role can change during its existence, but if they cannot, you might consider making them final to eliminate the possibility of involuntary modification of these properties. And if they can change, you'd have to make sure that these changes are reflected in the properties of a Composition object, which your code doesn't seem to take care of, so you already have an example of why storing a property in multiple places is dangerous.