Is it OK to have an empty method?

Auberon picture Auberon · Jun 1, 2015 · Viewed 11.3k times · Source

Is it OK to have an empty method and have it overridden in its subclass(es)? This is how it would look like in my code.

public class Rook() {

    public void voidCastleRight() { }

}

public class ShortRook() extends Rook {

    @Override
    public void voidCastleRight() {
        getPlayer().setkSC(false); //void King Side Castling (Short Castle)
    }

}

public class LongRook() extends Rook {

    @Override
    public void voidCastleRight() {
        getPlayer().setqSC(false); //void Queen Side Castling (Long Castle)
    }
}

The context is a chess engine. It is written in Java and it will have to search for the next 'best' move to make given a state of the board. It is therefore important that everything is implemented as efficient as possible because a lot of methods will be called several millions of times. Consequently, I want this hierarchy of Rooks as opposed to one Rook class in which I would have to check on what side the Rook is on and checking if the Rook is on its initial position etc etc..

When a Board is first created, there will be a ShortRook and a LongRook. While the game progresses, it is possible that more Rooks are introduced into the game because of Pawn promotion. These will be instantiations of Rook.

The method voidCastleRight() will be called whenever a Rook is moved. Rooks that exist because of Pawn promotion shouldn't void the castle right when moved (empty method). Rooks that exist since the beginning of the game, should void the castle rights when moved (methods in subclasses).

I have also written a parser which takes FENStrings and converts them to a Board and vice versa. When the Rooks are not on their initial positions, there is no way to tell which are the Short- and LongRook. This isn't a problem because the castle right is voided already anyway and they can be parsed to instantiations of Rook. Therefore would it be OK if I casted a Short- or LongRook object to Rook whenever it has voided the relevant castle right (i.e. it has moved)? This way it won't needlessly void the castle right when it is already voided. I don't care about the complexity of these parser methods as they will not be used in search.

While some might consider these thoughts micro-optimizations "which are the root of all evil", maybe these optimizations will pay off when the method has to be called a few million times. I'm also more concerced about OOP paradigms.

PS: I know Java isn't the best language to use for this application, it is irrelevant. I am aware that object creation (in Java) is expensive. I will make sure no objects are created during the search.

Answer

Sergey Kalinichenko picture Sergey Kalinichenko · Jun 1, 2015

Although it is certainly OK to have an empty method that does nothing, you should evaluate its alternative - an empty method that is missing entirely, i.e. an abstract method.

This may be applicable in your case, because you will be creating a ShortRook, a LongRook, or a promoted Rook:

public abstract class AbstractRook() {
    public abstract void voidCastleRight();
}

public class ShortRook() extends AbstractRook{
    @Override
    public void voidCastleRight() {
        getPlayer().setkSC(false); //void King Side Castling (Short Castle)
    }
}

public class LongRook() extends AbstractRook{
    @Override
    public void voidCastleRight() {
        getPlayer().setqSC(false); //void Queen Side Castling (Long Castle)
    }
}

public class PromotedRook() extends AbstractRook{
    @Override
    public void voidCastleRight() {
        throw new IllegalStateException("Promoted rook cannot castle");
    }
}