in Java, I am creating a class, and I would like to keep track of all objects the are created in this class. I have implemented a way to store the names of each object (using ArrayList), but I cannot figure out how to store the object itself in the ArrayList. Here is a snippet of my code:
static public class Ship {
private static ArrayList<String> ships = new ArrayList<String>();
private static ArrayList<Ship> shipObs = new ArrayList<Ship>();
String name;
private ArrayList<String> cruises = new ArrayList<String>();
int maxPassengers;
private static final String[] CABINS = new String[] {"Balcony", "Ocean View", "Suite", "Interior"};
private int[] passengers = new int[] {0,0,0,0};
boolean inService = false;
public Ship(String name, int maxPassengers) {
// Ensure that each ship has a unique name
if (ships.size() == 0) {
this.name = name;
ships.add(name);
}
else if (ships.size() >= 1) {
for (int i=0; i < ships.size(); i ) {
if (ships.get(i).equals(name)) {
System.out.println("Ship " name " cannot be created because that name already exists");
return;
}
}
this.name = name;
ships.add(name);
}
this.maxPassengers = maxPassengers;
As you can see, I have the static ArrayList that I would like to populate with all created ships. I assume that this population would take place in the initializing function, but the only method for doing so that I can see would to do something like
shipObs.add(this);
But that doesn't work...
CodePudding user response:
As pointed out in the comments to the question, in addition to the problem asked about, there is another problem: That is, the premature return
inside the constructor. Both problems can be addressed by calling the constructor indirectly, through a static
method.
import java.util.ArrayList;
public final class Ship {
// private static ArrayList<String> ships = new ArrayList<String>();
private static ArrayList<Ship> shipObs = new ArrayList<Ship>();
String name;
private ArrayList<String> cruises = new ArrayList<String>();
int maxPassengers;
private static final String[] CABINS =
new String[]{"Balcony", "Ocean View", "Suite", "Interior"};
private int[] passengers = new int[]{0, 0, 0, 0};
boolean inService = false;
private Ship(String name, int maxPassengers) {
this.name = name;
// shipObs.add (this);
this.maxPassengers = maxPassengers;
}
public static Ship createAShip(String name, int maxPassengers) {
for (int i = 0; i < shipObs.size(); i ) {
if (shipObs.get(i).name.equals(name)) {
System.out.println("Ship " name
" cannot be created because that name already exists");
return shipObs.get(i);
}
}
Ship theShip = new Ship(name, maxPassengers);
ShipObs.add (theShip);
return theShip;
}
}
I made the constructor private
. This prevents client code (code that uses the class) from calling the constructor, forcing use of the static
method. But, this disables inheritance. To make it clear that inheritance is not allowed, I added final
to public class Ship
.
As stated in the comments to the question, a constructor should not return
before construction of the Object is complete. If a constructor discovers it cannot be completed, an Exception
needs to be thrown.
The static method first checks for a duplicate Ship
name
. If found, it returns the Ship
that bears that name. It would be a good idea to change that part of the code to throw an exception. A third option is to have it return null
. Whatever the choice, it should be made clear to users of the class. This can be done using Javadoc.
If a duplicate name is not found, a new Ship
is created and returned.
I also simplified the code that checks for a duplicate name. If shipObs.size()
returns zero, the for
loop is not executed. It is not necessary to guard by enclosing within an if
.
I also removed ArrayList<String> ships
. Since an Object in shipObs
has a name
field, ArrayList<String> ships
is redundant.
CodePudding user response:
You can't just use a return
in the constructor, that would just avoid the next parameter association, you needs to throw an error to stop the instanciation process
If you may need the full objects in a list, track them only with a List<Ship>
You'll the simplified if/else
system, you can try to iterate over an empty, it doesn't matter, so just do it
class Ship {
private static List<Ship> ships = new ArrayList<>();
String name;
int maxPassengers;
public Ship(String name, int maxPassengers) {
this.name = name;
for (Ship ship : ships) {
System.out.println(ship " " ship.equals(this));
if (ship.equals(this)) {
String msg = "Ship " ship.name " cannot be created because that name already exists";
System.out.println(msg);
throw new IllegalArgumentException(msg);
}
}
ships.add(this);
this.maxPassengers = maxPassengers;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Ship ship = (Ship) o;
return name.equals(ship.name);
}
}
If you don't care about the full objects, just use a List<String>
private static List<String> shipNames = new ArrayList<>();
public Ship(String name, int maxPassengers) {
for (String ship : shipNames) {
if (ship.equals(name)) {
String msg = "Ship " name " cannot be created because that name already exists";
System.out.println(msg);
throw new IllegalArgumentException(msg);
}
}
this.name = name;
shipNames.add(name);
this.maxPassengers = maxPassengers;
}
The following code will throw the exception at the second line
Ship a = new Ship("ship_1", 10);
Ship b = new Ship("ship_1", 10);
Ship c = new Ship("ship_2", 10);