How to introduce my pokemon's Moves as Objects?

[Raed Tabani]

I'm building a pokemon battle simulator, as I mentioned in my previous post, and I'm starting from simplest game form and then adding complexities. right now I'm trying to give my pokemon multiple moves
to randomly(for now) choose from. and I started with an int[] array that holds the magnitude of each move.
but then it occurred to me that moves need to have multiple attributes like name, type,damage..etc and also they can be acquired by more than one pokemon, and having them as an array would mean literally that I would have to make multiple arrays and it would involve a lot of repetitiveness.
so I settled on Objects, however the first challenge that faced me was, how to introduced the different types of moves? one one hand, I have healing moves,which acts on the pokemon's own health. on the other hand, I have attacking moves,which acts on the other pokemon's health.
given that both types of moves need to be in the same object array, I thought of polymorphism and created a superclass "Moves", which has HealingMoves and FightingMoves as subclasses, so both can be sent to the Moves[]. but since these moves have different effects, I had to make an new if statement that checks for class of move selected and based on that does different things. I want your feedback guys on how the game's logic is going so far? is this the right way to introduce moves?

P.S this is somewhat out of topic, but what is the difference between these two statements:
1-System.out.println(user.moveList[2].getClass().equals(HealingMoves.class)
2-System.out.println(user.moveList[2].equals(HealingMoves.class)
statement two always evaluated to false for some reason...

GameClass

public class OutPut {

    static Monster user=new Monster("Me");
    static Monster playerOne;
    static Monster playerTwo;
    static Monster android=new Monster("android");
    static Scanner userFeed=new Scanner(System.in);
    public static void turnSelection()
    {
        System.out.println("1.Head or 2.Tails?");
        int headTails=(int)(Math.random()*2)+1;
        int userChoice=userFeed.nextInt();
        if(userChoice==headTails){
            System.out.println("you guessed right!, you go first");
            playerOne=user;
            playerTwo=android;
        }
        else {
            System.out.println("you guessed wrong!, android goes first");
            playerOne = android;
            playerTwo=user;
        }

    }


    public static void fight()
    {
        int randomAttack;
        while(true)
        {
            randomAttack=(int)(Math.random()*3);
            if(playerOne.moveList[randomAttack]
                      .getClass().equals(HealingMoves.class))
            {
                playerOne.health=playerOne.health+
                      playerOne.moveList[randomAttack].damage;
                System.out.println(playerOne.name+
                     " choose "+playerOne.moveList[randomAttack].name+
                     " which healed"+playerOne.moveList[randomAttack].damage);
            }
            else
            {
                playerTwo.health=
                        playerTwo.health-playerOne.moveList[randomAttack].damage;
                System.out.println(playerOne.name+
                       " choose "+playerOne.moveList[randomAttack].name+
                       " which dealt"+playerOne.moveList[randomAttack].damage+ "damage");
                    if(playerTwo.health<=0)
                    {

                        System.out.println(playerOne.name+" wins!");
                        break;

                    }
            }
            randomAttack=(int)(Math.random()*3);
            if(playerTwo.moveList[randomAttack]
                    .getClass().equals(HealingMoves.class))
            {
                playerTwo.health=playerTwo.health+
                     playerTwo.moveList[randomAttack].damage;
                System.out.println(playerTwo.name+
                    " choose "+playerTwo.moveList[randomAttack].name+
                    " which healed"+playerTwo.moveList[randomAttack].damage);
            }
            else {
                playerOne.health = playerOne.health -
                       playerTwo.moveList[randomAttack].damage;
                System.out.println(playerTwo.name + " choose " +
                       playerTwo.moveList[randomAttack].name + " which dealt" +
                        playerTwo.moveList[randomAttack].damage + "damage");
                if (playerOne.health <= 0) {

                    System.out.println(playerTwo.name + " wins");
                    break;
                }
            }
        }
    }
    public static void main(String[]args)
    {
//System.out.println(user.moveList[2].getClass().equals(HealingMoves.class));
        turnSelection();
        fight();
    }

}

MonsterClass

/**
 * Created by Raed on 3/6/2015.
 */
public class Monster {
    String name;
    int health=100;
    int[] attackList={10,20,15};
    Moves[] moveList={new FightingMoves("bite",10),
                new FightingMoves("fire",20),new HealingMoves("zamzam",20)};

    public Monster(String name)
    {
        this.name=name;
    }
}

Moves SuperClass

/**
 * Created by Raed on 3/6/2015.
 */
public class Moves {
    String name;
    int damage;


}

HealingMoves Class

public class HealingMoves extends Moves {

    public HealingMoves(String name,int damage)
    {
        this.name=name;
        this.damage=damage;
    }
}

FightingMoves Class

public class FightingMoves extends Moves {

    public FightingMoves(String name,int damage)
    {
        this.name=name;
        this.damage=damage;
    }
}

[Campbell Ritchie]

I have got rid of the very long lines and some of the blank lines which make your code difficult to read. You can see above how to do it
Please space your code out: there should be spaces before and after binary operators like +.

If you try getClass().equals…, you are testing whether the two objects were created from the same Class<?> object. Two Monsters might be created from the same Class<?> object so that sometimes evaluates to true. If you try whether the Monster object equals a particular Class<?> object, that will never be true.

But you should not be checking that sort of thing. There is something very wrong with checking the type of something; it seems to indicate the objects have not been designed polymorphically. You should have Attack objects, and you can either supply subclasses or an enumerated type representing different instances of Attack.

[Raed Tabani]

Im terribly sorry about that , i dont know why but whenever i open a curly brace my hand autmatically smashes enter i guess my mind is perparing me for the whole lot of code to come but not to worry, i will keep that in mind next time.

As for .getClass() does it mean that it returns an object of the calling class and compare it with object given in .equals()? if thats the case wouldnt that mean i will have these free floating objects everytime the statment is executed, which is alot, just for comparison reasons??
Would it be a better idea to create objects for comparison reasons only?


For me, the reason i used .getClass.equal() is that i needed a way to check " which subClass does this refrence index point to" is it a healingmove or a fightingmove as different actions are to be taken.

[Knute Snortum]

Instead of .getClass().equals, look into the instanceof operator.

if ( monster instanceof Ogre ) { ...

[Raed Tabani]

Instance of looks more appealing that all that .getClass().equalto() but how is it different that getClass? can we say this is always better?

[Knute Snortum]

In most cases, instanceof is better. It will tell you if the object is the class or subclass of what you're testing, and it will not give you a NullPointerException (NPE) if the object is null -- it will just return false.

[Raed Tabani]

Thanks man, it did the trick for me although im wondering why is checking the type of sth is considered bad practise?

[Knute Snortum]

I think the idea is that you want the maximum flexibility in your code, and instanceof will work even with nulls and subclasses. Others may have more thoughts on this.

[Junilu Lacar]

Raed Tabani wrote:Thanks man, it did the trick for me although im wondering why is checking the type of sth is considered bad practise?

Using instanceof is always a red flag to me in application code. Library utility code not so much but in application code where polymorphism is also a goal, definitely a sign that there is something off with the design.

I think the Moves as a class design actually sounds like it might be aligned with the Command Pattern and it could work but not the way you have implemented it right now. I'd have to play around with actual code to see for myself though.

[Junilu Lacar]

The trouble with OPs current design is that a Move does not represent encapsulated behavior but rather it is focused on the name and damage attributes. Also, the main code breaks the encapsulation of these Move objects by reaching in directly to access these values in the fight() method. Both these are counter to the goals of object orientation.

A Move has to know about Monster. The Monster in turn needs to provide some way(s) to allow a Move to affect it.

Something like this is how I would do it (caveat: I have not tested this!):

    public abstract class Move {
         private final String name;
         private final int effect;

         Move(String name, int effect) {
              this.name = name;
              this.effect = effect;
         }

         ...
 
         public final int getEffect() {
              return this.effect;
         }
  
         public abstract void applyTo(Monster monster);
    }

    public MinorHealing extends Move {
          ...
          
         @Override
         public void applyTo(Monster monster) {
              monster.heal(getEffect());
         }
    }

    public FullHealing extends Move {
         ...
         @Override
         public void applyTo(Monster monster) {
              monster.rejuvenate();
         }
    }

    public GlancingBlow extends Move {
         ...
         @Override
         public void applyTo(Monster monster) {
              monster.injure(getEffect());
         }
    }

    public FatalBlow extends Move {
         ...
         @Override
         public void applyTo(Monster monster) {
              monster.die();
         }
    }

The idea is to try to keep information encapsulated and only allow it pass to other classes as parameters to methods. Getters often break encapsulation and direct access to member attributes, as is the case in the OP's fight() method, is definitely breaking encapsulation.

The other thing too is that OOP is mostly about behaviors. If you find your class more concerned about the data it encapsulates then there is a good chance that you're not thinking in an object-oriented way.

To use this code polymorphically, without the need to use instanceof, you'd do something like this:

// somewhere in main code
List<Move> moves =  ...;
addDefensiveMoves(moves);
addAttackingMoves(moves);
player.addAll(moves);

// somewhere in Player:
Monster enemy = ...
for (Move attack : attackMoves) {
    attack.applyTo(enemy);
}

...
// heal thyself
for (Move repair: healthMoves) {
    repair.applyTo(this);
}

[Campbell Ritchie]

Raed Tabani wrote: . . . whenever i open a curly brace my hand autmatically smashes enter . . .

That is correct. The problem is not using enter later on. You need to push the enter key more frequently. Then your code would be easier to read.
And don't say sth.

[Campbell Ritchie]

Raed Tabani wrote: . . . As for .getClass() does it mean that it returns an object of the calling class . . .

No. I have already told you what getClass() does.

[Campbell Ritchie]

Raed Tabani wrote: . . . Would it be a better idea to create objects for comparison reasons only?

Not sure I understand that, but it doesn't sound good.

For me, the reason i used .getClass.equal() is that i needed a way to check " which subClass does this refrence index point to" is it a healingmove or a fightingmove as different actions are to be taken.

And as several people myself included have already told you, that is poor design.

[Junilu Lacar]

I started to edit my last post but to illustrate that constant refactoring and redesign is necessary when you are developing, I'll add more here. Things could get even more interesting if you want a Move to be applicable to both Monster and Player. Then you'll have to start thinking about interfaces. I would next try this:

public interface Curable {
    /** Repairs some damage **/
    void heal(int power);

    /** Restores all power **/
    void rejuvenate();
}

public interface Damageable {
    /** Incur some damage **/
    void injure(int power);

    /** Incur total loss of health **/
    void kill()
}

That would probably lead me to think there is really not much distinction between a Player and a Monster in terms of dealing with Moves; they are just objects that implement one or both of these interfaces. There could be more interfaces you could think of, like Mobile:

public interface Mobile {
    void jump(int howHigh);
    void dodge(Direction direction);
    void run(Direction direction, int intensity);
    void walk(Direction direction, int intensity);
    void swim(Direction direction, int intensity);
}

// then:
public class Combatant implements Mobile, Damageable, Curable {
    ...
}

// in main
Combatant player = new Player(...);
Combatant monster = new Monster(...);

One thing about programming is that you have to allow yourself to make "mistakes". Once you see your ideas in code and learn more about the interactions of these ideas in your program -- Move, Player, Monster, etc. are all just ideas, really -- you have to be able to say "Well, this doesn't work, let me try something else." By "working" I don't mean simply that the code compiles and runs. That doesn't mean much. Good working code and designs are easy: they're easy to read, easy to work with, easy to understand. It's when we try to force the code and design to do unnatural things that we get a lot of resistance and things get complicated, convoluted, and not fun. Forcing code and designs into conforming by using language features like instanceof is counterproductive. Even if you get the code to compile and the program to run reasonable well or even correctly, the code and design will still not be good.

So when, you might ask, is something like instanceof appropriate then? In certain implementations of equals(), where it makes sense, is one example I can give off the top of my head. In library utilities and framework code, maybe. Very rarely in application program code though, especially when the goal is polymorphism.

[Campbell Ritchie]

ArrayList#trimToSize and #ensureCapcaity are example where you have to use instanceof in your code. Think how much better it would be if they had empty methods in the List interface for those things. Now Java8 is here they could add them simply with empty methods

public interface List<E>
{
  //...
   /**
    * blahblahblah
    *@param i ...
    */
   public default void ensureCapacity(int i)
   {
      // empty method
   }
  //...
   /**
    * blahblahblah
    */
   public default void trimToSize()
   {
      // empty method
   }
  //...
}