0

I asked a question about using the Strategy pattern and LSP, but the answer were mixed, and in the long run, it isn't good idea. However a comment at the bottom of my question stated that while I'm not in violation of LSP, I'm in violation of the Interface Segregation Principle which states:

that clients should not be forced to implement interfaces they don't use. Instead of one fat interface many small interfaces are preferred based on groups of methods, each one serving one submodule.

Suppose I had a Weapon class like this:

public final Weapon{

    private final String name;
    private final int damage;
    private final List<AttackStrategy> validactions;
    private final List<Actions> standardActions;

    public Weapon(String name, int damage, List<AttackStrategy> standardActions, List<Actions> attacks)
    {
        this.name = name;
        this.damage = damage;
        standardActions = new ArrayList<Actions>(standardActions);
        validAttacks = new ArrayList<AttackStrategy>(validActions);
    }

    public void standardAction(String action){} // -- Can call reload or aim here.  

    public int attack(String action){} // - Call any actions that are attacks. 

}

Interface and implementation:

public interface AttackStrategy{
    void attack(Enemy enemy);
}

public class Shoot implements AttackStrategy {
    public void attack(Enemy enemy){
        //code to shoot
    }
}

public class Strike implements AttackStrategy {
    public void attack(Enemy enemy){
        //code to strike
    }
}

Use:

List<AttackStrategy> actions = new ArrayList<AttackStrategy();
actions.add(new Shoot())

List<Actions> standardActionactions = new ArrayList<Actions>();
actions.add(new Reload(10))

Weapon rifle = new Weapon("Sniper Rifle", 5, standardActionactions, actions);

List<AttackStrategy> actions = new ArrayList<AttackStrategy();
actions.add(new Swing())

List<Actions> standardActionactions = new ArrayList<Actions>();
actions.add(new Drop())

Weapon sword = new Weapon("Standard Sword", 10, standardActionactions, actions);

I have two Weapons, and both behave differently, while both can be used to attack, both have different actions specific to each type. For example, Sword shouldn't and by the logic of my collection, cannot call or implement reload in anyway.

In my text adventure game if you pass "reload" to sword.standardAction("reload"), the internal logic will check to see if the List contains a reload object, and if does, do it, if not, the weapon isn't capable and therefore can't, so it ignore it.

Am I in violation of the ISP?

My sword is never going to implement reload, unless a client decides to implement it, I don't force a client to accept that a Sword is aware of the Reload method which they will never use.

  • You are not really violating the ISP, but you also aren't taking advantage of the type system. How about dropping a bit of flexibility in order to define common abstractions? E.g. all weapons could have `reload`, `lightAttack`, `normalAttack`, `specialAttack` and an associated strategy for each. – plalx Oct 23 '17 at 21:19
  • @plalx - My main issue is, why would sword be aware of reload? and if Sword overrides reload and leaves it blank, doesn't that violate the LSP? Either way it seems I'm violating some principal, just ignoring it to get a game with multiple weapons. –  Oct 23 '17 at 21:36
  • @plalx - Given that I've followed the rules of the site, and tried for myself, how would you solve it without violating LSP/ISP or any other SOLID principal? –  Oct 23 '17 at 21:41

1 Answers1

0

It sounds like that the Weapon abstraction is a little too general. I would break that into smaller abstractions

  1. Weapon (abstract or interface - take your pick)

    • attack
  2. GunBasedWeapon (extends Weapon) (guns, rifles, laser guns, 80s megatron)

    • aim
    • reload
    • attack (throws OutOfBulletException)
  3. HandheldWeapon (extends Weapon) (swords, clubs, chains, etc)

    • attack (throws CallousesOnHandHurtException after 100 hits without lotion)
    • applyLotion
  4. ThrowableWeapon (extends Weapon) (boomerangs)

    • attack (throws SomebodyElseCaughtMyBoomerangException, and then ImHosedException after that)
  5. WeaponAction - each weapon class can now have a list of valid actions, if somehow someone passes "applyLotion" to a GunBasedWeapon, only the Gun class will know that it's not valid.

Goose
  • 546
  • 3
  • 7
  • Thanks for this, but I wanted to avoid casting, which if I use this pattern I would have to, to access class specific methods. Isn't casting something you'd want to avoid? –  Oct 24 '17 at 12:27