3
\$\begingroup\$

I'm learning Java with Head First Java.

I've built a program that creates a 7x7 grid (A0 to G6) and places an object of 3 consecutive values inside it. The user has to guess the location and when the 3 values are found, the program will confirm that the object is destroyed.

My problem right now is that I'm not satisfied with my current implementation, even if it works.

In the DotComBust class, where the main sits, I had created those 3 consecutive values on a 7x7 grid and then I assigned them to an ArrayList field in DotCom. Then, I had asked the user for an input and his input is sent to a method, which traverses this ArrayList field and removes this value from the list if it's found there. Once all 3 values are knocked out, the program concludes that the object has been killed and outputs the number of guesses.

There are 96 lines of code that correspond to the creation of this grid (lines 14 to 115). I think that this is too much and I'm not sure if my logic is wrong, the choice of statements or both.

DotCom.java

import java.util.ArrayList;

public class DotCom {

    private ArrayList<String> locationCells;
    private int numOfHits = 0;

    public DotCom() {

    }

    public void setLocationCells(ArrayList<String> locs) {
        locationCells = locs;
    }

    public String checkYourself (String stringGuess) {
        String result = "miss";

        int index = locationCells.indexOf(stringGuess);
        if (index >= 0) {
            locationCells.remove(index);
            if (locationCells.isEmpty()) {
                result = "kill";
            } else {
                result = "hit";
            }
        }
        System.out.println(result);
        return result;
    }

}

DotComBust.java (the main and where the "grid" is created)

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

public class DotComBust {

    public static void main(String[] args) {

        int numOfGuesses = 0;

        DotCom dot = new DotCom();

        int firstLocationVInteger = (int) (Math.random() * 7);
        int firstLocationH = (int) (Math.random() * 7);
        int secondLocationVInteger = 0;
        int thirdLocationVInteger = 0;
        int secondLocationH = 0;
        int thirdLocationH = 0;
        int directionChoice;
        if (((firstLocationVInteger >= 0) && (firstLocationVInteger <= 4)) && ((firstLocationH >= 0) && (firstLocationH <= 4))) {
            // 0 for down, 1 for right
            directionChoice = (int) (Math.random() * 2);
            if (directionChoice == 0) {
                secondLocationVInteger = firstLocationVInteger + 1;
                thirdLocationVInteger = firstLocationVInteger + 2;
                secondLocationH = firstLocationH;
                thirdLocationH = firstLocationH;
            } else {
                secondLocationH = firstLocationH + 1;
                thirdLocationH = firstLocationH + 2;
                secondLocationVInteger = firstLocationVInteger;
                thirdLocationVInteger = firstLocationVInteger;
            }
        }
        else {
            // 0 for up, 1 for left
            directionChoice = (int) (Math.random() * 2);
            if (directionChoice == 0) {
                secondLocationVInteger = firstLocationVInteger - 1;
                thirdLocationVInteger = firstLocationVInteger - 2;
                secondLocationH = firstLocationH;
                thirdLocationH = firstLocationH;
            } else {
                secondLocationH = firstLocationH - 1;
                thirdLocationH = firstLocationH - 2;
                secondLocationVInteger = firstLocationVInteger;
                thirdLocationVInteger = firstLocationVInteger;
            }
        }

        String firstLocationV;
        switch (firstLocationVInteger) {
            case 0:     firstLocationV = "A";
                break;
            case 1:     firstLocationV = "B";
                break;
            case 2:     firstLocationV = "C";
                break;
            case 3:     firstLocationV = "D";
                break;
            case 4:     firstLocationV = "E";
                break;
            case 5:     firstLocationV = "F";
                break;
            case 6:     firstLocationV = "G";
                break;
            default:    firstLocationV = "Invalid range";
                break;
        }

        String secondLocationV;
        switch (secondLocationVInteger) {
            case 0:     secondLocationV = "A";
                        break;
            case 1:     secondLocationV = "B";
                        break;
            case 2:     secondLocationV = "C";
                        break;
            case 3:     secondLocationV = "D";
                        break;
            case 4:     secondLocationV = "E";
                        break;
            case 5:     secondLocationV = "F";
                        break;
            case 6:     secondLocationV = "G";
                        break;
            default:    secondLocationV = "Invalid range";
                        break;
        }

        String thirdLocationV;
        switch (thirdLocationVInteger) {
            case 0:     thirdLocationV = "A";
                        break;
            case 1:     thirdLocationV = "B";
                        break;
            case 2:     thirdLocationV = "C";
                        break;
            case 3:     thirdLocationV = "D";
                        break;
            case 4:     thirdLocationV = "E";
                        break;
            case 5:     thirdLocationV = "F";
                        break;
            case 6:     thirdLocationV = "G";
                        break;
            default:    thirdLocationV = "Invalid range";
                        break;
        }

        //concatenate
        String firstLocation = firstLocationV + Integer.toString(firstLocationH);
        String secondLocation = secondLocationV + Integer.toString(secondLocationH);
        String thirdLocation = thirdLocationV + Integer.toString(thirdLocationH);

        ArrayList<String> locations = new ArrayList<>();
        locations.add(firstLocation);
        locations.add(secondLocation);
        locations.add(thirdLocation);
        dot.setLocationCells(locations);

        Scanner sc = new Scanner(System.in);

        boolean isAlive = true;
        while (isAlive) {
            System.out.print("Enter your guess: ");
            String userGuess = sc.next();
            String result = dot.checkYourself(userGuess);
            numOfGuesses++;

            if (result.equals("kill")) {
                    isAlive = false;
                    System.out.println("You took " + numOfGuesses + " guesses.");
            }
        }

    }
}

Output sample

...
Enter your guess: F0
miss
Enter your guess: F1
miss
Enter your guess: F2
miss
Enter your guess: F3
miss
Enter your guess: F4
miss
Enter your guess: F5
miss
Enter your guess: F6
miss
Enter your guess: G0
miss
Enter your guess: G1
hit
Enter your guess: G2
hit
Enter your guess: G3
kill
You took 47 guesses.
\$\endgroup\$

3 Answers 3

3
\$\begingroup\$
  1. all switch-case statements are the same, just invoked on different variables -> put the statement in a method and pass it the variable to query. Moreover, the whole switch-case statement can be replaced by getting the index of the int-location from the String "ABCDEFG":
    locationStr = "ABCDEFG".substring(locationInt, locationInt+1); and you can catch the IndexOutOfBoundsException to detect out of range int value

  2. you already use a collection to put locations. instead of keeping three separate set of variables for V/H and the rest-> use a colection for these as well.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you, I've just read about utility classes and passing the statement that previously went into switch to a method from there certainly helps. I've now got: String firstLocationV = GameHelper.intToString(firstLocationVInteger); String secondLocationV = GameHelper.intToString(secondLocationVInteger); String thirdLocationV = GameHelper.intToString(thirdLocationVInteger); in the main and the switch is stored in the intToString method of the GameHelper utility class. I'll look at your other suggestions too. \$\endgroup\$
    – Dorkymon
    Commented Apr 10, 2018 at 12:40
2
\$\begingroup\$

A little late to the party, but here's some thoughts.

For this type of project, I find a great simplicity in representing each spot on the grid with a class(Cell). Since the size of the board is known before the game starts, a simple 2D array of type Cell is perfect for representing the board. It is a fairly trivial matter to convert a readable representation of the coordinates to the array indexes.

The next class I like to make is a Location class to represent the coordinates of each cell. The main advantage of this, you not only can name the coordinate parts(row,col), you can very easily include the location as a field in the Cell class.

One thing I noticed where you could really use some improvement is deciding on the direction the target will lie. Consider that the direction can be represented by offsets of the start location. If we consider the start location with offset 0,0, then towards the top and to the right would be -1,1, etc.

-------------------
|-1,-1|-1, 0|-1, 1|
-------------------
| 0,-1| 0, 0| 0, 1|
-------------------
| 1,-1| 1, 0| 1, 1|
-------------------

To keep setting cells in that direction, simple add the offset to each coordinate for however many cells you want to set. Take a random integer returning any value from 0 - 2 and subtract one to get a random offset for one coordinate.

\$\endgroup\$
0
\$\begingroup\$

Assuming that all three blocks are the same you can fill the grid like this:

    int firstLocationVInteger = 0;
    int secondLocationVInteger = 0;
    int thirdLocationVInteger = 0;
    int firstLocationH = 0;
    int secondLocationH = 0;
    int thirdLocationH = 0;

    int firstLocNum1 = (int) (Math.random() * 5);
    int firstLocNum2 = (int) (Math.random() * 7);

    if ((int) (Math.random() * 2) == 0) { //horizontal
        firstLocationVInteger = firstLocNum2;
        secondLocationVInteger = firstLocNum2;
        thirdLocationVInteger = firstLocNum2;
        firstLocationH = firstLocNum1;
        secondLocationH = firstLocNum1 + 1;
        thirdLocationH = firstLocNum1 + 2;
    } else { //vertical
        firstLocationVInteger = firstLocNum1;
        secondLocationVInteger = firstLocNum1 + 1;
        thirdLocationVInteger = firstLocNum1 + 2;
        firstLocationH = firstLocNum2;
        secondLocationH = firstLocNum2;
        thirdLocationH = firstLocNum2;
    }

This can be further improved by using a collection for the location as already mentioned.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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