Design for container that holds multiple instances of same base class

Question

I have four different classes. First being a base class for the second and third. The fourth being a container class that holds instances of second and third (stored as first).

My base class for PC and Companion

public class Controllable
{
 protected String name;
 protected int x;
 public Controllable (String name) {
 this.name = name;
 }
 public void move () {
 x++;
 }
}
public class PC extends Controllable
{
 private int someUniqueVariable;
 public PC (String name, int someUniqueVariable) {
 super (name);
 this.someUniqueVariable = someUniqueVariable;
 }
}
public class Companion extends Controllable
{
 public Companion (String name) {
 super (name);
 }
}

And finally the wrapper/container class.

public class Party
{
 private PC pc;
 private List<Controllable> members;
 public Party () {
 PC = new PC ("PC", 1); // edit #1: should be pc, bad copy-paste
 members = new ArrayList<Controllable>();
 members.add (PC);
 members.add (new Companion ("Companion1");
 members.add (new Companion ("Companion2");
 }
 public moveAll () {
 for (Controllable m : members) {
 m.move();
 }
 }
 public getPC () {
 return pc;
 }
}

Now if I want to move all Controllable I will iterate over members. If there are pc specifics I will simply call the PC and use its move function. PC will be more separated from Companion later on.

I would mostly like feedback on the design of my code.

Edit #1:

The most critical area were I want feedback is this. Most things are shared between Companion and PC, however there are some unique features of PC. In 99% I want to be able to iterate over all of the party members In more rare cases I want to only access the Companions and sometimes I only want to access the PC.

public Party () {
 pc = new PC ("PC", 1);
 members = new ArrayList<>();
 members.add (PC);
 members.add (new Companion ("Companion1");
 members.add (new Companion ("Companion2");
}

Minor edit to show an error in the code.

• CBredlow: Looks good, however, would it be possible to make an array of Companions a property of PC? That is only assuming that the Companions will always follow the PC.
• Emz: @CBredlow, Companions will always follow the PC, but not vice-verse. So possible yes, however doesn't that break the design a bit? Trying to move those parts to Party.
• CBredlow: I don't think it would, but it was just a thought. It may add more work so it might not be a good idea to do my suggestion.

Answer 1

Controllable class

• can be abstract, it doesn't seem you will instantiate it
• int x field shall be private, you don't use it anywhere

Party class

• you declared variable of type PC with lower letters so object assignment shall be too pc = new PC ("PC", 1);
• members = new ArrayList<Controllable>(); with Java 8 it is not necessary to declare list's type if you do this with variable declaration, so members = new ArrayList<>(); is enough

It is a good practice to make method arguments final, as well class fields if they are provided through constructor. It means their declared type cannot be changed.

• rolfl: Hi, and welcome to Code Review. Good spotting for the PC -> pc bug. That's what happens when people try to 'edit' or 'fix' their code before posting to Code Review.... it leads to unexpected bugs. The Diamond operator <> is available in Java7 too, FYI.
• Emz: The ´abstract´ part makes sense. The int x is just a temporary variable. The PC = new PC ("PC", 1); is a typo. Basically everything is temporary in this code apart from the storing. I should emphasize that a bit more I guess.
• Emz: @Leszek, I have never ever seen final in method arguments. Do you have perhaps have some good examples of where I can read more about that?
• Leszek Gruchała: Lots of resources: javapractices.com/topic/TopicAction.do?Id=23 , stackoverflow.com/questions/154314/… , stackoverflow.com/questions/137868/… , programmers.stackexchange.com/questions/48413/…