Is it common to use static void method to handle objects, e.g. set some properties? [closed]

Question

I'm trying to develop a text-based game. Here is the concept-demo version.

class Arena {
    public static void battle(Hero h1, Hero h2){
        h1.setHp(h1.getHp()-h2.getPhysical_attack());
        h1.info();
    }

    public static void main(String[] args) {
        System.out.println("Entering Arena ...");
        Hero mike = new Hero();
        mike.setUname("mike");
        mike.info();
        Hero tom = new Hero();
        tom.setUname("tom");
        tom.info();
        battle(mike, tom);
    }
}

class Hero{
    private String uname;
    private int hp;
    private int physical_attack;
    Hero(){
        hp = 620;
        physical_attack = 66;
    }
    public String getUname() {
        return uname;
    }

    public void setUname(String uname) {
        this.uname = uname;
    }

    public int getHp() {
        return hp;
    }

    public void setHp(int hp) {
        this.hp = hp;
    }

    public int getPhysical_attack() {
        return physical_attack;
    }

    public void setPhysical_attack(int physical_attack) {
        this.physical_attack = physical_attack;
    }
    void info() {
        System.out.println("HP of "+uname+"'s hero = "+hp);
    }
}

Is it common to use static void method to handle objects that way?

Answer 1

Few suggestions

1. Arena.battle can be changed to instance method rather than a static method. This will help in conducting matches in different arenas.
2. Use constructor to set the uname, initial hp and initial physical_attack
3. Its better to use camel case for variable names instead of underscore(_)
4. Constructors can be overloaded to mandate uname and suitable default values for hp and physicalAttack.
5. toString() can be overloaded instead of info()
6. hashCode and equals can be overridden to ensure that same player is not matched against himself

Regarding the usage of static method to schedule games

1. It's always better to have a separate scheduler class to schedule games.
2. This will help in unit testing using dependency injection
3. also, it will be easier to mock non-static methods than static methods in unit test

Answer 2

Since the Arena class has no state, and what state it needs is passed in as arguments, it seems reasonable to me for the methods to be static. This is a common pattern for utility-like classes. To make it more explicit you can add the keyword final to the class to indicate that no-one should create an instance of this class (there would be no point).

However the way you're accessing that method is not very standard. If you want it to be static you should really be accessing it like so:

Arena.battle(mike, tom);

If, as others have suggested, you want Arena to be a class from which instance objects are created, rather than a utility class, then I'd suggest giving it some state. Your battle method already takes two Hero instances. An alternative approach would be to set the Hero instances on your Arena object and then call battle like so:

class Arena {

    private Hero h1;
    private Hero h2;

    public void battle() {
        h1.setHp(h1.getHp()-h2.getPhysical_attack());
        h1.info();
    }

    public static void main(String[] args) {
        System.out.println("Entering Arena ...");

        Hero mike = new Hero();
        mike.setUname("mike");
        mike.info();

        Hero tom = new Hero();
        tom.setUname("tom");
        tom.info();

        Arena arena = new Arena();
        arena.hero1 = mike;
        arena.hero2 = tom;
        arena.battle();
    }
}

That way you can call other methods on the Arena in the future and not have to remember which Heros you passed to it. For example, a method that prints the result of the battle, or an average result of multiple battles over time.