Adding Created Object to ArrayList During Initialization - Java

Question

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...

Answer 1

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.

Answer 2

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);