Constant Instance Variables vs Inheritance

[Gregg Bolinger]

I wanted to get myself back into some core OO concepts and away from all this CRUD POJO HTML JS CSS mess I deal with on a daily basis where the most logic I have to write is checking for null values. I've been going through some tutorials online for developing various little games because I like games and they are good for solving logical problems.

I'm writing a small console based battleship game (I know, cutting edge) and I am facing a bit of an OO issue when creating the Ship object. I can think of a couple good ways to handle it.

1. One of the tutorials I went through had this for the Ship object.

public class Ship
{
    public static final int TYPE_NONE = -1;
    public static final int TYPE_AIRCRAFT_CARRIER = 0;
    public static final int TYPE_BATTLESHIP = 1;
    public static final int TYPE_CRUISER = 2;
    public static final int TYPE_SUBMARINE = 3;
    public static final int TYPE_PT_BOAT = 4;
    public static final int ORIENTATION_UP = 0;
    public static final int ORIENTATION_RIGHT = 1;
    public static final int ORIENTATION_DOWN = 2;
    public static final int ORIENTATION_LEFT = 3;
 
    // Instance variables.
    private int type;
    private int orientation;
    private int row;
    private int column;
    private int size;
    ....
}

So when creating a new ship I have a constructor that takes a value for the type, orientation, row, col, and size. The type and orientation would be one of the class constants available.

2. Inheritance

public abstract class Ship
{
    // common attributes associated with a ship
    private int row;
    private int col;
    ....
    public abstract void someSpecificMethodAboutSpecificShip();
    
}

public class Battleship extends Ship
{
   
    public void someSpecificMethodAboutSpecificShip()
    {
         
    }
}

Currently, I can't think of anything that is so specific about a Battleship vs a Ship that has the Battleship attribute to warrant the extra class. However, I can't help but feel that solution #1 is less OO than solution #2.

Opinions?
[ November 09, 2006: Message edited by: Gregg Bolinger ]

[Frank Carver]

I would definately go with an O-O approach.

If you were to continue coding the first approach, eventually you would probably end up with some code like:

public void sunk() {
  System.out.print("you sunk my " + name());
}
 
private String name() {
  switch(type) {
    case TYPE_AIRCRAFT_CARRIER: return "Aircraft Carrier";
    case TYPE_BATTLESHIP: return "Battleship";
    case TYPE_CRUISER: return "Cruiser";
    case TYPE_SUBMARINE: return "Submarine";
    case TYPE_PT_BOAT: return "PT Boat";
  }
  return "open sea?";
}

There would likely also be a similar switch for the dimensions of the vessel. Looking at the constants for orientation, there would even be some sort of switching to work out which map locations the vessel occupies.

These are prime candidates for polymorphism. As a first alternative, imagine your base vessel class has two methods:

protected abstract String name();
protected abstract Dimension size();

Your subclasses extends the base class, and fills in the abstract methods:

class Battleship extends Vessel {
  protected String name() {
    return "Battleship";
  }
 
  protected Dimension size() {
    return new Dimension(3,2);
  }
}

To use these classes you might say something along the lines of:

Armada armada = new Armada();
armada.add(new AircraftCarrier());
armada.add(new Battleship());
...

This is the kind of O-O design you will see in many textbooks and tutorials. In practice, I mostly do a slightly different design, as I prefer to minimise the number of classes in the system together with replacing switches and ifs with polymorphism wherever I can.

class vessel {
  private String name;
  private Dimension size;
 
  public Vessel(String name, Dimension size) {
    this.name = name;
    this.size = size;
  }
 
  public Vessel(String name, int width, int height) {
    this(name, new Dimension(width, height);
  }
 
  public Vessel(String name, int width) {
    this(name, width, 1);
  }
 
  public void rotate(int steps) {
    // a little math to swap and/or negate the width and height of the size
  }
  ...
 
  public void sunk() {
    System.out.println("you sunk my " + name);
  }
}

To use this approach, just pass appropriate contructor parameters when creating the vessels:

Armada armada = new Armada();
armada.add(new Vessel("Aircraft Carrier", 3, 2);
armada.add(new Vessel("Battleship", 5);
...

The important thing is that in the two alternative solutions, there is no need for any of those constant values, and no need for long and clumsy switches or if trains.

Does that make any sense?

[Tim LeMaster]

I might be tempted to do this

    
public abstract class Ship
{
//REPLACE ME WITH AN ENUM FOR JavaSE 5
public static final int VERTICAL = 0;
public static final int HORIZONTAL = 0;

private int row, column, orientation;

public Ship(int row, int column, int orientation)
{
this.row = row;
this.column = column;
this.orientation = orientation;
}

public abstract int getSize();

public int getRow()
{
return row;
}

public int getColumn()
{
return column;
}

public int getOrientation()
{
return orientation;
}
}

public class BattleShip extends Ship
{
public BattleShip(int row, int column, int orientation)
{
super(row, column, orientation);
}

public int getSize()
{
return 5;
}
} 

That way the size of the ship is not determined the caller. I also assume the position is the upper most or left most point so only Vertical and Horizontal is needed.

[Ilja Preuss]

If you want to hide the actual ship size from the caller (which I think would make sense if instances are created at more than one place), I probably still wouldn't use subclassing to do so. I'd think that simply using creation methods would be simpler.

[Tim LeMaster]

Yeah I was thinking when developing the game you would probably find more differences like Frank's name field. And its a console game now but an image field. Or a sound field (that plays when it gets sunk - YOU SANK MY BATTLESHIP!).

Of course if I was going this far I'd probably put the ship definitions in an embedded DB or properties file and have a factory.

Ship battleShip = shipFactory.get("Battleship");

Then its a matter of changing the DB or properties files to control the size, images, etc of a ship.

[Gregg Bolinger]

There would likely also be a similar switch for the dimensions of the vessel. Looking at the constants for orientation, there would even be some sort of switching to work out which map locations the vessel occupies.

Oh yea. Actually, the example I found was using if statements and I changed them to switch statements. But they are everywhere. Checking for collisions when placing the AI ships and checking for out of bounds is quite verbose and ver procedural. I would rather deal with Object placement than int placement, if that makes sense. However, in every task maybe the best solution isn't always the standard practice?

This is the kind of O-O design you will see in many textbooks and tutorials.

When text books are trying to teach OOP, I see this. When tutorials and books are trying to teach you how to accomplish a task (ie, make a battleship game) I don't see this. Hence, my original question.

If you want to hide the actual ship size from the caller (which I think would make sense if instances are created at more than one place), I probably still wouldn't use subclassing to do so. I'd think that simply using creation methods would be simpler.

How so? By this do you mean you would go the route of my example #1?

I like Tim's approach in that the implemented ships have class name different from one another. The reason I like this approach a bit better than just declaring a new Vessel and passing it a ship name is because if I think down the road I can imagine a battleship game where there are more differences between the different types of ships and I'd want fine grained classes to represent those, while still sharing similar ship characteristics.

[Gregg Bolinger]

Something else I noticed that is a bit flawed in the #1 design...Whenever the code has to check for collision with another ship before placement, the routine traverses the entire Board array[][] and looks for an existing ship at a specific [r][c]. I'd rather just check the ship going to be placed against the ships already on the board. That seems faster to me than going through the entire 10x10 grid (which I can imagine in future versions being much bigger).

I don't think solution #1 gives me a good way to accomplish this. But I haven't looked into it much this morning yet. I am focusing on refactoring it into a Tim/Frank hybrid.

[Frank Carver]

How so? By this do you mean you would go the route of my example #1?

Um, I'd use code like my final example. Did you miss it ?

[Gregg Bolinger]

Originally posted by Frank Carver:
How so? By this do you mean you would go the route of my example #1?

Um, I'd use code like my final example. Did you miss it ?

Sorry, this was directed to Ilja.

[Ilja Preuss]

    
public class Ship
{
 
private Ship(String name, int size, int row, int column, int orientation)
{
            ...
}


public static Ship createBattleship(int row, int column, int orientation)
{
return new Ship(BATTLESHIP_NAME, BATTLESHIP_SIZE, row, column, orientation);
}
}

That way, client code can create battleship instances without knowing details about it, but you don't yet need subclasses. If you later find out that the design might benefit from subclasses, you can introduce them without any changes to client code.

This is the kind of set based decision making we were talking about in the Process forum this week. Decoupling the clients from the design details allows us to actively defer the final decision on whether to use subclasses or not.