0

I was working on a problem not too long ago, and the topic of responsibilities and where they should go came up. In my linked problem, the University is the institution and class that holds all the rules and ensures that each student registering, has the required documents.

This made me recall a blog post by Eric Lippert: Wizards and Warriors.

It reads:

We keep on talking about “rules”, and so apparently the business domain of this program includes something called a “rule”, and those rules interact with all the other objects in the business domain. So then should “rule” be a class? I don’t see why not! It is what the program is fundamentally about. It seems likely that there could be hundreds or thousands of these rules, and that they could change over time, so it seems reasonable to encode them as classes.

The Rules:

  • A warrior can only use a sword.

  • A wizard can only use a staff.

My problem is how would I determine in the GameRules class the specific object being passed to verify if I can carry it?

public final class GameRules {

    public static boolean verifyifwizardcancarry(Weapon weapon){
        boolean canCarry  = false
        if weapon is a a staff set canCarry to true
        return canCarry;
    }
 }

public abstract class Player{   

   private List<Weapon> weapons;
   // constructor and OOP goodness left out

   public abstract void add(Weapon weapon);

}


public final class Wizard extends Player{

   @Override 
   public void add(Weapon weapon){

      if(GameRules.verifyifwizardcancarry(weapon){
          // - code to add weapon to inventory
      }
    }
 }

How can I verify that the Weapon being passed to the GameRules class is in fact a Sword? From reading, the use of instanceof or getClass() is considered code smell.

I found this answer, but it violates LSP and goes against what the blog states, that it shouldn't throw an exception, and doesn't really answer my question.

  • 3
    Why do you care if it's a `Sword`? If it is, it surely has something different from all the other implementations of `Weapon`. Or even better, if your problem is whether it can be carried, why don't `Weapon`s have a property `canBeCarried`? – Federico klez Culloca Mar 02 '18 at 15:32
  • @FedericoklezCulloca Wizard is just one implementation of player (btw, implement Player in Wizard) and the role determines available weapons I'm guessing – Lance Toth Mar 02 '18 at 15:40
  • The `Visitor` pattern in [this blog](https://dzone.com/articles/instanceof-considered-harmful) may be of use to you. That said, `instanceof` remains a bit of a philosophical debate, because while it can definitely be used in smelly ways, it also has its legitimate uses (default `equals()` implementations for starters). – Brandon McKenzie Mar 02 '18 at 15:49
  • @LanceToth - Corrected, it extends Player now. Made a mistake when posting –  Mar 02 '18 at 15:57
  • @FedericoklezCulloca - As pointed out in the blog post, `Weapon` shouldn't have to worry about whose carrying it, the Rules state what character is allowed to carry what item. –  Mar 02 '18 at 16:05
  • I gave the example in the wrong way: what I mean is that the `Sword` should have a property that `Player` should use to establish if the `Weapon` is carriable, not that the `Weapon` decides whether it can be carried. – Federico klez Culloca Mar 02 '18 at 16:17
  • @FedericoklezCulloca - Problem is, when you have rules that state only a `Wizard` can carry a staff and only a `Warrior` can carry a `Sword`, how do you enforce those rules? This is what the series of blog posts attempts to solve. Creating a `Rule` class is one way, but you have to know specifically what you're carrying. I can have `Sword` implement a method called isCarriable, but then it has to be aware of whose carrying it, which it shouldn't, not it's responsibility too, it's the `Rules` responsibility. Feel free to provide a full answer :D –  Mar 02 '18 at 16:24

2 Answers2

0

As per this Java dynamic binding and method overriding

Creating a function like this would be called when a Sword as added, as opposed to any other child of Weapon

public void add(Sword sword){
    //nope
}

This can be done also if only one (or a few) weapons can be equipped, but then the Overriden function would give a negative response.

Lance Toth
  • 430
  • 3
  • 17
  • But then you'd be moving the `instanceof` check from inside `Player.add` to some other enclosing object that is passing out weapons to all the players. – George Aristy Mar 02 '18 at 16:07
0

I don't think you can avoid the instanceof operator in your case. What I suggest is that you make each concrete Player on its own responsible for determining what weapons it can carry. So remove GameRules.verifyifwizardcancarry and spread this logic out between the players such that Wizard does instanceof checks for its weapons, and other players do the same.

George Aristy
  • 1,373
  • 15
  • 17
  • This is probably the best answer, but I'm going to wait a little longer before checking, who knows Eric Lippert himself may help! –  Mar 02 '18 at 16:33
  • @Nexusfactor: Sadly I do not have time today to weigh in. Maybe next week. :-) – Eric Lippert Mar 02 '18 at 17:55