5
\$\begingroup\$

I am a beginner in the programming world. This is the first big project I tackle which is the battleships game. I implemented the game to run in the console. I was hoping if you guys can check my code and help realize if I can have shortcuts or make it more efficient.

The game goes as follows:

  1. deploy map.
  2. user enters coordinates of his ships.
  3. random coordinates are generated for pc ships.
  4. the user ships should be on top of each other and the same goes for the pc ships.
  5. the pc and the user ships cannot be on top of each other as well.
  6. now the game starts, the user enter coords if he misses the "O" appears, if he hits the "!" appears, if he hits his ship the "X" appears.
  7. if he hits pc/his ships the user can't enter the same coords again. he cant enter the coords of the ones he missed too.
  8. random coords are generated for the pc, but need to make sure that the numbers were not already used and missed and it can't generate the same coords when it hits its-own/user ships.

I had to use an array list to store where the pc has missed whereas for the user I am visually comparing the "O". I am not sure how I can test the part where the pc can't enter the missed coords again. Finally, can I get some tips or help with improving the pieces of my code and make it more efficient?

UPDATE : I rethought the layout of my code, so i could use fewer functions that can be used by both the user and the pc but at the price of complicating things a bit. This is the final version, do you think something can be fixed to make it more efficient?

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Scanner;

public class BattleShips {


  // INITIALIZE MAIN VARIABLES AND CALL FUNCTIONS IN ORDER
  public static void main(String[] args) {

    // VARIABLES //
    Scanner input = new Scanner(System.in);
    String[][] battleMap = new String[10][10];
    Integer[][] userShips = new Integer[5][2];
    Integer[][] pcShips = new Integer[5][2];
    Integer[] dummyCoords = new Integer[2];
    ArrayList<ArrayList<Integer>> pcMissedShips = new ArrayList<ArrayList<Integer>>();

    // FUNCTION CALLS //
    intro();
    fillBattleMap(battleMap);

    userShips = getUserShips(input, userShips, 0);
    manipulateBattleMapShips(userShips, battleMap, dummyCoords, "@", true);
    updateBattleMap(battleMap);

    System.out.println("         Computer is deploying ships");
    pcShips = getPcShips(pcShips, userShips, 0);
    manipulateBattleMapShips(pcShips, battleMap, dummyCoords, " ", true);
    updateBattleMap(battleMap);

    while (!isGameOver(battleMap)) {

        userCoordsEntry(input, battleMap, pcMissedShips, userShips, pcShips);
        pcCoordsEntry(battleMap, pcMissedShips, userShips, pcShips);
        updateBattleMap(battleMap);
    }
}


 // INTRODUCTION
 public static void intro() {

    System.out.println();
    System.out.println("***** Let's play a game of Battle Ships *****");
    System.out.println();
}


 // FILL BATTLEMAP ARRAY AND PRINT
 public static void fillBattleMap(String[][] battleMap) {

    System.out.println("        0  1  2  3  4  5  6  7  8  9");
    System.out.println("       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");

    for (int i = 0; i < battleMap.length; i++) {

        System.out.print("    " + i + " | ");

        for (int j = 0; j < battleMap[i].length; j++) {

            battleMap[i][j] = " ";
            System.out.print(battleMap[i][j] + "  ");
        }

        System.out.print("| " + i);
        System.out.println();
    }

    System.out.println("       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
    System.out.println("        0  1  2  3  4  5  6  7  8  9");
    System.out.println();
}


 // PRINT BATTLEMAP
 public static void updateBattleMap (String[][] battleMap) {

    System.out.println();
    System.out.println("        0  1  2  3  4  5  6  7  8  9");
    System.out.println("       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");

    for (int i = 0; i < battleMap.length; i++) {

        System.out.print("    " + i + " | ");

        for (int j = 0; j < battleMap[i].length; j++) {

            System.out.print(battleMap[i][j] + "  ");
        }

        System.out.print("| " + i);
        System.out.println();
    }

    System.out.println("       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
    System.out.println("        0  1  2  3  4  5  6  7  8  9");
    System.out.println();
}


 // USER INPUTS SHIPS LOCATION AND RETURNS 2D ARRAY CONTAINING COORDINATES
 public static Integer[][] getUserShips (Scanner input, Integer[][] userShips, int loop) {

    boolean checkIfOutOfBounds = false;
    boolean checkIfThereIsShipForUser = false;
    Integer[] userShipsCoords = new Integer[2];

    for (int i = loop; i < 5; i++) {

        System.out.print("Enter X coordinate for your " + (i + 1) + ". ship:");
        userShipsCoords[0] = input.nextInt();
        System.out.print("Enter Y coordinate for your " + (i + 1) + ". ship:");
        userShipsCoords[1] = input.nextInt();

        checkIfOutOfBounds = checkIfOutOfBounds(userShipsCoords);
        checkIfThereIsShipForUser = checkIfThereIsShip(userShipsCoords, userShips);

        if (checkIfOutOfBounds) {

            return getUserShips(input, userShips, i);

        } else if (checkIfThereIsShipForUser) {

            return getUserShips(input, userShips, i);
        }

        userShips[i][0] = userShipsCoords[0];
        userShips[i][1] = userShipsCoords[1];
    }
    return userShips;
}


 // RANDOMS SHIPS LOCATIONS AND MAKES SURE THEY DO NOT COLLIDE WITH THE USER'S OR PC'S SHIPS
 public static Integer[][] getPcShips (Integer[][] pcShips, Integer[][] userShips, int loop) {

    Integer[] pcCoords = new Integer[2];
    boolean checkIfThereIsShipForPc = false;
    boolean checkIfThereIsShipForUser = false;

    for (int i = loop; i < pcShips.length; i++) {

        pcCoords[0] = (int) Math.floor(Math.random()*10);
        pcCoords[1] = (int) Math.floor(Math.random()*10);

        checkIfThereIsShipForUser = checkIfThereIsShip(pcCoords, userShips);
        checkIfThereIsShipForPc = checkIfThereIsShip(pcCoords, pcShips);

        if (checkIfThereIsShipForPc) {

            return getPcShips(pcShips, userShips, i);

        } else if (checkIfThereIsShipForUser) {

            return getPcShips(pcShips, userShips, i);
        }

        pcShips[i][0] = pcCoords[0];
        pcShips[i][1] = pcCoords[1];
        System.out.println("              " + (i+1) + ". Ship DEPLOYED ");
    }
    System.out.println("              ----------------");

    return pcShips;
}


 // TAKE USER COORDINATES INPUT AND CHECKS IF ITS VALID THEN UPDATES BATTLEMAP ARRAY, IF NOT THE USER HAS TO ENTER AGAIN NEW COORDS
 public static Integer[] userCoordsEntry (Scanner input, String[][] battleMap,  ArrayList<ArrayList<Integer>> pcMissedShips, Integer[][] userShips, Integer[][] pcShips) {

    Integer[] userCoords = new Integer[2];

    System.out.println();
    System.out.println("Your Turn.");
    System.out.print("Enter X coordinate:");
    userCoords[0] = input.nextInt();
    System.out.print("Enter Y coordinate:");
    userCoords[1] = input.nextInt();

    if (checkIfOutOfBounds(userCoords)) {

        System.out.println("Choose again");
        return userCoordsEntry (input, battleMap, pcMissedShips, userShips, pcShips);
    }
    else if (checkIfUserShipIsHitAtCoords(battleMap, userCoords)) {

        System.out.println("Choose again");
        return userCoordsEntry (input, battleMap, pcMissedShips, userShips, pcShips);
    }
    else if (checkIfPcShipIsHitAtCoords(battleMap, userCoords)) {

        System.out.println("Choose again");
        return userCoordsEntry (input, battleMap, pcMissedShips, userShips, pcShips);
    }
    else if (checkIfShipIsMissed(battleMap, userCoords, true, pcMissedShips)) {

        System.out.println("Choose again");
        return userCoordsEntry (input, battleMap, pcMissedShips, userShips, pcShips);
    }
    else {

        if (checkIfThereIsShip (userCoords, userShips)) {

            System.out.println("You hit your own ship...");
            manipulateBattleMapShips (userShips, battleMap, userCoords, "X", false);
        }
        else if (checkIfThereIsShip (userCoords, pcShips)) {

            System.out.println("BOOM! You got your opponent!");
            manipulateBattleMapShips (pcShips, battleMap, userCoords, "!", false);
        }
        else {

            System.out.println("OH no! You missed:(");
            manipulateBattleMapShips (userShips, battleMap, userCoords, "O", false);
        }
    }
    return userCoords;
}


 // RANDOMS COMPUTERS COORDS INPUT AND CHECKS IF ITS VALID THEN UPDATES BATTLEMAP ARRAY, IF NOT THE USER HAS TO ENTER AGAIN NEW COORDS
 public static Integer[] pcCoordsEntry (String[][] battleMap, ArrayList<ArrayList<Integer>> pcMissedShips, Integer[][] userShips, Integer[][] pcShips) {

    Integer[] pcCoords = new Integer[2];

    System.out.println();
    System.out.println("Computer's Turn.");
    pcCoords[0] = (int) Math.floor(Math.random()*10);
    pcCoords[1] = (int) Math.floor(Math.random()*10);
    System.out.println("The Computer chose it\'s X coordinate to be: " + pcCoords[0]);
    System.out.println("The Computer chose it\'s Y coordinate to be: " + pcCoords[1]);

    if (checkIfUserShipIsHitAtCoords(battleMap, pcCoords)) {

        return pcCoordsEntry (battleMap, pcMissedShips, userShips, pcShips);
    }
    else if (checkIfPcShipIsHitAtCoords(battleMap, pcCoords)) {

        return pcCoordsEntry (battleMap, pcMissedShips, userShips, pcShips);
    }
    else if (checkIfShipIsMissed(battleMap, pcCoords, false, pcMissedShips)) {

        return pcCoordsEntry (battleMap, pcMissedShips, userShips, pcShips);
    }
    else {

        if (checkIfThereIsShip (pcCoords, userShips)) {

            System.out.println("NO! Computer hit your ship.");
            manipulateBattleMapShips (userShips, battleMap, pcCoords, "|", false);
        }
        else if (checkIfThereIsShip (pcCoords, pcShips)) {

            System.out.println("YES! Computer hit his own ship");
            manipulateBattleMapShips (pcShips, battleMap, pcCoords, "#", false);
        }
        else {

            System.out.println("Computer missed:)");
            pcMissedShips.add(new ArrayList<Integer>(Arrays.asList(pcCoords)));
        }
    }

    return pcCoords;
}


 // END GAME CONDITION
 public static boolean isGameOver (String[][] battleMap) {

    int pcShipCount = 5;
    int userShipCount = 5;

    for (int i = 0; i < battleMap.length; i++) {

        for (int j = 0; j < battleMap[i].length; j++) {

            if (battleMap[i][j].equals("X")) {

                userShipCount--;
            }

            if (battleMap[i][j].equals("|")) {

                userShipCount--;
            }

            if (battleMap[i][j].equals("#")) {

                pcShipCount--;
            }

            if (battleMap[i][j].equals("!")) {

                pcShipCount--;
            }
        }
    }

    if (userShipCount == 0) {

        System.out.println("GAME OVER! Computer Wins");
        System.out.println("Your ships: " + userShipCount + " | Comuter ships " + pcShipCount);
        return true;
    }
    else if (pcShipCount == 0) {

        System.out.println("GAME OVER! You win");
        System.out.println("Your ships: " + userShipCount + " | Comuter ships " + pcShipCount);

        return true;
    }

    return false;
}


 // MANIPULATE BATTLEMAP BY "CHARACTER" WITH ANOTHER "CHARACTER"
 public static void manipulateBattleMapShips (Integer[][] ships, String[][] battleMap, Integer[] coords, String character, boolean addShipOrAddCharacter) {

    if (addShipOrAddCharacter) {

        for (int i = 0; i < ships.length; i++) {

            battleMap[ships[i][1]][ships[i][0]] = character;
        }
    }
    else {

        battleMap[coords[1]][coords[0]] = character;
    }
}


 // CHECK IF THE COORDINATES OF SHIP ARE INSIDE THE MAP
 public static boolean checkIfOutOfBounds (Integer[] coords) {

    if (coords[1] < 0 || coords[1] > 9 || coords[0] < 0 || coords[0] > 9) {

        return true;
    }
    else {

        return false;
    }
}


 // CHECK IF THE SHIPS LOCATIONS ARE NOT DUPLICATED
 public static boolean checkIfThereIsShip (Integer[] coords, Integer[][] ships) {

    for (int i = 0; i < ships.length; i++) {

        if (coords[0] == ships[i][0] && coords[1] == ships[i][1]) {

            return true;
        }
    }

    return false;
}


 // CHECK IF USER SHIP IS HIT WHEN COORDINATES ARE ENTERED
 public static boolean checkIfUserShipIsHitAtCoords (String[][] battleMap, Integer[] coords) {

    if (battleMap[coords[1]][coords[0]].equals("|") || battleMap[coords[1]][coords[0]].equals("X")) {

        return true;
    }
    else {

        return false;
    }
}

 // CHECK IF PC SHIP IS HIT WHEN C
 public static boolean checkIfPcShipIsHitAtCoords (String[][] battleMap, Integer[] coords){

    if (battleMap[coords[1]][coords[0]].equals("#") || battleMap[coords[1]][coords[0]].equals("!")) {

        return true;
    }
    else {

        return false;
    }
}


 // CHECK IS USER OR PC SHIPS ARE MISSED AT COORDINATES ENTERED/GUESSED
 public static boolean checkIfShipIsMissed (String[][] battleMap, Integer[] coords, boolean userOrPc,  ArrayList<ArrayList<Integer>> pcMissedShips) {

    if (userOrPc) {

        if (battleMap[coords[1]][coords[0]].equals("O")) {

            return true;
        }
        else {

            return false;
        }
    }
    else {

        if (pcMissedShips.size() == 0) {

            return false;
        }

        for (int i = 0; i < pcMissedShips.size(); i++) {

            if (coords[0] == pcMissedShips.get(i).get(0) && coords[1] == pcMissedShips.get(i).get(1)) {

                return true;
            }
        }

        return false;
    }
}

}

\$\endgroup\$
1
\$\begingroup\$

Welcome to the world of programming! Congrats on undertaking your first project as well! I will have to spend more time looking at your code to figure out how to do things more efficiently, but in terms of adding functionality to prevent a user from entering duplicate coordinates I think you can expand your checkOutOfBounds() function.

I think you can rename that function to be checkIfValidInput(). This function will not only check the out of bounds condition but also check to see if a set of coordinates has already been entered. Are you familiar with ArrayList? This would be a good opportunity to use one to keep track of the coordinates entered by the user. You can either use an array list (preferable because the size can dynamically increase) or an array of 2-element arrays , i.e. an array of each user coordinate choice during the game. When the user enters a set of coordinates inside of your userCoordsChoice(), you store that coordinate and when you call the checkIfValidInput() you can check this array to see if those coordinates have already have been called.

\$\endgroup\$
  • \$\begingroup\$ I think i got flustered with the amount of tasks to be done, so i decided to have function for each task to make it simpler (don't know if that is the correct approach). As for the user input i do not need to track that in an array list because i can display on the map where i already tried to search for the opponent's ships with an "O" so i search the battleMap array for that string. Same approach goes to the rest except the only that cannot work this way, being the attempts of the pc because i am not allowed to visualize that.The arraylist is hard to test to see if its working. \$\endgroup\$ – FHajj Aug 30 '18 at 3:58
  • 2
    \$\begingroup\$ @FHajj As a rule of thumb, functions should do one thing and one thing only. So putting every task in it's own function is perfectly valid. But you can put functions in functions and re-use functions. \$\endgroup\$ – Mast Aug 30 '18 at 5:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.