ArrayList doesn't add objects the inteded way

Question

I'm trying to add multiple values to an ArrayList via an if/else statement. The variable numberOfShips declares how many as a field. However, when I print out .size(), the size is only 1, with the latest added object being the one in there. I can't figure out what I'm doing wrong. I know ArrayList increases implicitly in size as elements are added, so that can't be it.

public ArrayList<Ship> createFleet(int choice) {
    ArrayList<Ship> fleet = new ArrayList<Ship>();
    if (count < numberOfShips && choice > 0 && choice < 5) {
        if (choice == 1) {
            Ship ac = new Ship("Aircraft carrier", 5, false);
            fleet.add(ac);
            count++;
            System.out.println("Aircraft carrier has been added to fleet.");
        } else if (choice == 2) {
            Ship bs = new Ship("Battleship", 4, false);
            fleet.add(bs);
            count++;
            System.out.println("Battleship has been added to fleet.");
        } else if (choice == 3) {
            Ship sm = new Ship("Submarine", 3, false);
            fleet.add(sm);
            count++;
            System.out.println("Submarine has been added to fleet.");
        } else if (choice == 4) {
            Ship ds = new Ship("Destroyer", 3, false);
            fleet.add(ds);
            count++;
            System.out.println("Destroyer has been added to fleet.");
        } else if (choice == 5) {
            Ship sp = new Ship("Patrol Boat", 2, false);
            fleet.add(sp);
            count++;
            System.out.println("Patrol boat has been added to fleet.");
        }
    } else {
        System.out.println("Not an option.");
    }
    return fleet;
}

Have you seen the switch statement? it might help you here. — vikingsteve
– vikingsteve, Commented Oct 13, 2014 at 13:27
If you are passing in the choice and expecting the method to add a ship to an existing list, you need to pass in the list instead of creating a new list in your method. — khelwood
– khelwood, Commented Oct 13, 2014 at 13:34

Narmer · Accepted Answer · 2014-10-13 14:04:32Z

You are creating a new ArrayList every time you call the method. You need to maintain an ArrayList of ships outside your method.

public ArrayList<Ship> createFleet(int choice) {
    ArrayList<Ship> fleet = new ArrayList<Ship>(); //Here you create a new ArrayList and return it with a single Ship in it.

You need a global scoped ArrayList variable:

private List<Ship> fleet = new ArrayList<Ship>();

public ArrayList<Ship> createFleet(int choice) {
    if (count < numberOfShips && choice > 0 && choice < 5) {
        if (choice == 1) {
            Ship ac = new Ship("Aircraft carrier", 5, false);
            fleet.add(ac);
            count++;
            System.out.println("Aircraft carrier has been added to fleet.");
        } else if (choice == 2) {
            Ship bs = new Ship("Battleship", 4, false);
            fleet.add(bs);
            count++;
            System.out.println("Battleship has been added to fleet.");
        } else if (choice == 3) {
            Ship sm = new Ship("Submarine", 3, false);
            fleet.add(sm);
            count++;
            System.out.println("Submarine has been added to fleet.");
        } else if (choice == 4) {
            Ship ds = new Ship("Destroyer", 3, false);
            fleet.add(ds);
            count++;
            System.out.println("Destroyer has been added to fleet.");
        } else if (choice == 5) {
            Ship sp = new Ship("Patrol Boat", 2, false);
            fleet.add(sp);
            count++;
            System.out.println("Patrol boat has been added to fleet.");
        }
    } else {
        System.out.println("Not an option.");
    }
    return fleet;
}

My suggestion is to create a Fleet class which contains the reference for the ArrayList:

public class Fleet {
    private List<Ship> internalFleet = new ArrayList<Ship>();
    private static int MAX_SHIPS = 10;

    public void addShip(int choice){
        if (internalFleet.size() < MAX_SHIPS) {
            Ship ship;
            switch(choice){
                case 1: ship = new Ship("Aircraft carrier", 5, false);
                        break;
                case 2: ship = new Ship("Battleship", 4, false);
                        break;
                case 3: ship = new Ship("Submarine", 3, false);
                        break;
                case 4: ship = new Ship("Destroyer", 3, false);
                        break;
                case 5: ship = new Ship("Patrol Boat", 2, false);
                        break;
                default: System.out.println("Not an option.");
            }
            if(ship!=null){
                internalFleet.add(ship);
                System.out.println(ship.getName() + " has been added to fleet.");
            }
        }
    }
    public ArrayList<Ship> getFleet(){
        return internalFleet;
    }
}

Eran · Accepted Answer · 2014-10-13 13:30:32Z

0

Your createFleet method only adds a single ship to the list, based on the input choice. If you want to add multiple ships, you'll to run that code in some loop. It's not clear why you are passing the choice to the method in the first place if you want multiple ships to be created.

If your intent was to create multiple ships of the same type, you can do :

ArrayList<Ship> fleet = new ArrayList<Ship>();
for (int count=0; count < numberOfShips ; count++) {
    if (choice == 1) ...
}

Your code can be further improved by replacing the long if .. else if ... else if with a switch statement.

edited Oct 13, 2014 at 13:30

answered Oct 13, 2014 at 13:25

Eran

395k57 gold badges726 silver badges793 bronze badges

6 Comments

Rasmus Over a year ago

If I put the code in the loop and i input choice, it chooses the same object numberOfShips times. I need to be able to choose different Ship objects

Eran Over a year ago

@user2740689 Where is the loop? If you have a loop that calls createFleet multiple times, each call would create a new list with a single ship.

Rasmus Over a year ago

It's in a different class the calls the method.

Rasmus Over a year ago

The loop is: while(battleship.count < battleship.numberOfShips) { fleet = new ArrayList<Ship> (battleship.createFleet(serverEntry.nextInt())); }

Eran Over a year ago

@user2740689 Then that's the problem. Your method creates a new empty list each time it is called.

|

uncleBounty · Accepted Answer · 2014-10-13 13:44:15Z

0

for(int count=0; count < numberOfShips ; count++){
    if (choice == 1) {
        fleet.add(new Ship("Aircraft carrier", 5, false));
        System.out.println("Aircraft carrier has been added to fleet.");
    }
    else if ...
}

answered Oct 13, 2014 at 13:44

uncleBounty

7938 silver badges14 bronze badges

Collectives™ on Stack Overflow

ArrayList doesn't add objects the inteded way

3 Answers 3

Comments

6 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

6 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related