1
\$\begingroup\$

You can set any size of the field and any number of bombs. I think I got a pretty flexible program again(Tic tac toe game for the console). What do you think about the code? I would like to know if I made mistakes in program design.

skeleton:

public class Minesweeper {
    public static void play(){
        final int HOWMANYBOMBS;
        boolean isItWin;
        Area[][] area;
        final Scanner SCANNER = new Scanner(System.in);
        final String greeting = "Hi, lets play minesweeper!";

        System.out.println(greeting);
        area = pickLengthsOfArea(SCANNER);
        HOWMANYBOMBS = getHowManyBombs(area,SCANNER);
        System.out.println("hi");
        fillArea(area,HOWMANYBOMBS);

        while(true) {
            printPole(area);
            if(playerTurnsAndIsPlayersTurnInBomb(area,SCANNER)) {
                isItWin=false;
                break;
            }
            if(weHaveNotGotAnyEmptyAreaThatNotCheckedAndWeMarkedOnlyBombs(area)) {
                isItWin=true;
                break;
            }
        }
        if(isItWin) {
            System.out.println("U won!");
        }
        else {
            System.out.println("Defieat!");
        }
    }

    private static boolean weHaveNotGotAnyEmptyAreaThatNotCheckedAndWeMarkedOnlyBombs(Area[][] area) {}

    private static boolean playerTurnsAndIsPlayersTurnInBomb(Area[][] area,Scanner scanner) {}

    private static void openAllAround(int x, int y, Area[][] area,ArrayList<AreaWithXandY> weOpenedThese) { // do you know another implementation?
        AreaWithXandY[] areasAround = getAllAreasAroundWithTheirXandY(area,x,y);
        for(AreaWithXandY a:areasAround) {
            if(!weOpenedThese.contains(a)) {
                a.getArea().setStatusOfArea(StatusOfArea.OPENED);
                weOpenedThese.add(a);
                if(a.getArea().getValueOfArea()==ValueOfArea.NOONEBOMBAROUND) {
                    openAllAround(a.getX(),a.getY(),area,weOpenedThese);
                }
            }
        }
    }

    private static void printPole(Area[][] area) {}

    private static void fillArea(Area[][] area, int howManyBombs) {
        fillAreaWithBombs(area,howManyBombs);
        fillAreaWithEmptyArea(area);
    }
    private static void fillAreaWithEmptyArea(Area[][] area) {}
    private static Area[] getAllAreasAround(Area[][] area,int x,int y) {}
    private static AreaWithXandY[] getAllAreasAroundWithTheirXandY(Area[][] area,int x,int y) {}
    private static void fillAreaWithBombs(Area[][] area, int howManyBombs) {}
    private static int getHowManyBombs(Area[][] area, Scanner scanner) {}

    private static Area[][] pickLengthsOfArea(Scanner scanner) {}

    private static boolean isXandYIn(int turnX,int turnY, Area[][] area) {}

    public static boolean isNumeric(String strNum) {}
}
class Area{
    private final ValueOfArea valueOfArea;

    private StatusOfArea statusOfArea;

    {
        statusOfArea = StatusOfArea.CLOSED;
    }
    Area(ValueOfArea valueOfArea){
        this.valueOfArea = valueOfArea;
    }
    ValueOfArea getValueOfArea() {
        return valueOfArea;
    }
    StatusOfArea getStatusOfArea() {
        return statusOfArea;
    }
    void setStatusOfArea(StatusOfArea statusOfArea) {
        this.statusOfArea = statusOfArea;
    }
}
class AreaWithXandY{
    private int x,y;
    Area area;

    AreaWithXandY(int x,int y,Area area){
        this.area = area;
        this.x = x;
        this.y = y;
    }
    int getX() {
        return x;
    }
    int getY() {
        return y;
    }
    Area getArea() {
        return area;
    }
    @Override
    public boolean equals(Object obj) {
        if(obj==null) {
            return false;
        }
        if(false==(obj instanceof AreaWithXandY)) {
            return false;
        }
        AreaWithXandY object = (AreaWithXandY) obj;
        if(object.getX()==x && object.getY()==y) { //its enouth
            return true;
        }
        else {
            return false;
        }
    }
}
enum StatusOfArea{
    MARKEDASBOMB,OPENED,CLOSED
}
enum ValueOfArea{
    BOMB("@"),
    NOONEBOMBAROUND("0"),ONEBOMBAROUND("1"),TWOBOMBAROUND("2"),TREEBOMBAROUND("3"),FOURBOMBAROUND("4"),FIVEBOMBAROUND("5"),SIXBOMBAROUND("6"),SEVENBOMBAROUND("7"),EIGHTBOMBAROUND("8");
    private String icon;
    ValueOfArea(String icon){
        this.icon = icon;
    }
    String getIcon() {
        return icon;
    }
    static ValueOfArea getValueOfAreaByString(String value) {
        ValueOfArea[] values = ValueOfArea.values();
        for(int i=0;i<values.length;i++) {
            if(values[i].getIcon().equals(value)) {
                return values[i];
            }
        }
        System.err.println("ERROR in getValueOfAreaByString(String value), we could not find this value");
        return null;
    }
}

fullcode:

public class Minesweeper {
    public static void play(){
        final int HOWMANYBOMBS;
        boolean isItWin;
        Area[][] area;
        final Scanner SCANNER = new Scanner(System.in);
        final String greeting = "Hi, lets play minesweeper!";

        System.out.println(greeting);
        area = pickLengthsOfArea(SCANNER);
        HOWMANYBOMBS = getHowManyBombs(area,SCANNER);
        System.out.println("hi");
        fillArea(area,HOWMANYBOMBS);

        while(true) {
            printPole(area);
            if(playerTurnsAndIsPlayersTurnInBomb(area,SCANNER)) {
                isItWin=false;
                break;
            }
            if(weHaveNotGotAnyEmptyAreaThatNotCheckedAndWeMarkedOnlyBombs(area)) {
                isItWin=true;
                break;
            }
        }
        if(isItWin) {
            System.out.println("U won!");
        }
        else {
            System.out.println("Defieat!");
        }
    }

    private static boolean weHaveNotGotAnyEmptyAreaThatNotCheckedAndWeMarkedOnlyBombs(Area[][] area) {
        for(int y = 0;y<area.length;y++) {
            for(int x = 0;x<area[0].length;x++) {
                if(area[y][x].getValueOfArea()!=ValueOfArea.BOMB/*is not bomb*/ && (area[y][x].getStatusOfArea()==StatusOfArea.CLOSED || area[y][x].getStatusOfArea()==StatusOfArea.MARKEDASBOMB) ) {
                    return false;
                }
            }
        }
        return true;
    }

    private static boolean playerTurnsAndIsPlayersTurnInBomb(Area[][] area,Scanner scanner) {
        while(true) {
            System.out.println("Print \"open x y\", if u want open this area, print \"mark x y\", if u want mark this area as bomb or unmark this area");
            String[] commandAndXAndY = scanner.nextLine().split(" ");
            if(commandAndXAndY.length!=3) {
                System.out.println("fill out the form correctly!");
            }
            else if(!commandAndXAndY[0].equals("open") && !commandAndXAndY[0].equals("mark")) {
                System.out.println("first work should be equal \"open\" or \"mark\"!");
            }
            else if(!isNumeric(commandAndXAndY[1]) || !isNumeric(commandAndXAndY[2])) {
                System.out.println("x and y should be numbers!");
            }
            else if(!isXandYIn(Integer.parseInt(commandAndXAndY[1]),Integer.parseInt(commandAndXAndY[2]),area)) {
                System.out.println("x and y should be in area! P.S.: area.lengthY="+area.length+", area.lengthX="+area[0].length);
            }
            else {
                int y = Integer.parseInt(commandAndXAndY[2]),x = Integer.parseInt(commandAndXAndY[1]);
                if(commandAndXAndY[0].equals("open")) {
                    if(area[y][x].getValueOfArea()==ValueOfArea.BOMB) {
                        return true;
                    }
                    else {
                        area[y][x].setStatusOfArea(StatusOfArea.OPENED);
                        if(area[y][x].getValueOfArea()==ValueOfArea.NOONEBOMBAROUND) {
                            openAllAround(x,y,area, new ArrayList<>());
                        }
                        return false;
                    }
                }
                else { // mark
                    area[y][x].setStatusOfArea(StatusOfArea.MARKEDASBOMB);
                    return false;
                }

            }

        }
    }

    private static void openAllAround(int x, int y, Area[][] area,ArrayList<AreaWithXandY> weOpenedThese) {
        AreaWithXandY[] areasAround = getAllAreasAroundWithTheirXandY(area,x,y);
        for(AreaWithXandY a:areasAround) {
            if(!weOpenedThese.contains(a)) {
                a.getArea().setStatusOfArea(StatusOfArea.OPENED);
                weOpenedThese.add(a);
                if(a.getArea().getValueOfArea()==ValueOfArea.NOONEBOMBAROUND) {
                    openAllAround(a.getX(),a.getY(),area,weOpenedThese);
                }
            }
        }
    }

    private static void printPole(Area[][] area) {
        System.out.println();
        for(int y = 0;y<area.length;y++) {
            for(int x=0;x<area[0].length;x++) {
                if(area[y][x].getStatusOfArea()==StatusOfArea.CLOSED) {
                    System.out.print("?");
                }
                else if(area[y][x].getStatusOfArea()==StatusOfArea.OPENED) {
                    System.out.print(area[y][x].getValueOfArea().getIcon());
                }
                else if(area[y][x].getStatusOfArea()==StatusOfArea.MARKEDASBOMB) {
                    System.out.print("*");
                }

            }
            System.out.println();
        }
    }

    private static void fillArea(Area[][] area, int howManyBombs) {
        fillAreaWithBombs(area,howManyBombs);
        fillAreaWithEmptyArea(area);
    }
    private static void fillAreaWithEmptyArea(Area[][] area) {
        for(int y = 0;y<area.length;y++) {
            for(int x=0;x<area[0].length;x++) {
                if(area[y][x] == null) {
                    int howManyBombsAround=0;
                    Area[] areasAround = getAllAreasAround(area,x,y);
                    for(Area a:areasAround) {
                        if(a!=null && a.getValueOfArea()==ValueOfArea.BOMB) {
                            howManyBombsAround++;
                        }
                    }
                    area[y][x] = new Area(ValueOfArea.getValueOfAreaByString(Integer.toString(howManyBombsAround)));
                }
            }
        }
    }
    private static Area[] getAllAreasAround(Area[][] area,int x,int y) {
        Area[] areasAround = new Area[8];
        int i=0;
        if(y!=0 && x!=0) { //left up
            areasAround[i] = area[y-1][x-1];
            i++;
        }
        if(y!=0) { // up
            areasAround[i] = area[y-1][x];
            i++;
        }
        if(x!=area[0].length-1 && y!=0) { //right up
            areasAround[i] = area[y-1][x+1];
            i++;
        }
        if(x!=0) { // left
            areasAround[i] = area[y][x-1];
            i++;
        }
        if(x!=area[0].length-1) { //right
            areasAround[i] = area[y][x+1];
            i++;
        }
        if(x!=0 && y!=area.length-1) { // left down
            areasAround[i] = area[y+1][x-1];
            i++;
        }
        if(y!= area.length-1) { //down
            areasAround[i] = area[y+1][x];
            i++;
        }
        if(x!=area[0].length-1 && y!=area.length-1) { // right down
            areasAround[i] = area[y+1][x+1];
            i++;
        }
        Area[] areasAroundWhithoutNullObjects = new Area[i];
        for(int b = 0;b<i;b++) {
            areasAroundWhithoutNullObjects[b] = areasAround[b];
        }
        return areasAroundWhithoutNullObjects;
    }
    private static AreaWithXandY[] getAllAreasAroundWithTheirXandY(Area[][] area,int x,int y) {
        AreaWithXandY[] areasAroundWithXandY = new AreaWithXandY[8];
        int i=0;
        if(y!=0 && x!=0) { //left up
            areasAroundWithXandY[i] = new AreaWithXandY(x-1,y-1,area[y-1][x-1]);
            i++;
        }
        if(y!=0) { // up
            areasAroundWithXandY[i] = new AreaWithXandY(x,y-1,area[y-1][x]);
            i++;
        }
        if(x!=area[0].length-1 && y!=0) { //right up
            areasAroundWithXandY[i] = new AreaWithXandY(x+1,y-1,area[y-1][x+1]);
            i++;
        }
        if(x!=0) { // left
            areasAroundWithXandY[i] = new AreaWithXandY(x-1,y,area[y][x-1]);
            i++;
        }
        if(x!=area[0].length-1) { //right
            areasAroundWithXandY[i] = new AreaWithXandY(x+1,y,area[y][x+1]);
            i++;
        }
        if(x!=0 && y!=area.length-1) { // left down
            areasAroundWithXandY[i] = new AreaWithXandY(x-1,y+1,area[y+1][x-1]);
            i++;
        }
        if(y!= area.length-1) { //down
            areasAroundWithXandY[i] = new AreaWithXandY(x,y+1,area[y+1][x]);
            i++;
        }
        if(x!=area[0].length-1 && y!=area.length-1) { // right down
            areasAroundWithXandY[i] = new AreaWithXandY(x+1,y+1,area[y+1][x+1]);
            i++;
        }
        AreaWithXandY[] areasAroundWithXandYWhithoutNullObjects = new AreaWithXandY[i];
        for(int b = 0;b<i;b++) {
            areasAroundWithXandYWhithoutNullObjects[b] = areasAroundWithXandY[b];
        }
        return areasAroundWithXandYWhithoutNullObjects;
    }
    private static void fillAreaWithBombs(Area[][] area, int howManyBombs) {
        ArrayList<Integer> listOfAllNumbers = new ArrayList<>(area.length*area[0].length);
        for(int i=0;i<area.length*area[0].length;i++) {
            listOfAllNumbers.add(i,i);
        }
        int tempId,y,x;
        for(int i=0;i<howManyBombs;i++) {
            tempId = listOfAllNumbers.get((int) (Math.random()*listOfAllNumbers.size()));
            listOfAllNumbers.remove(new Integer(tempId));
            y = ((int)tempId/area[0].length);
            x = tempId%area[0].length;
            area[y][x] = new Area(ValueOfArea.BOMB);
        }
    }
    private static int getHowManyBombs(Area[][] area, Scanner scanner) {
        while(true) {
            System.out.println("print number of bombs: ");
            String howManyBombsString = scanner.nextLine();
            if(!isNumeric(howManyBombsString)) {
                System.out.println("it should be number!");
            }
            else if(false==(0<Integer.parseInt(howManyBombsString) && Integer.parseInt(howManyBombsString)<area.length*area[0].length)) {
                System.out.println("it should be positive and it should not exceed the field capacity!");
            }
            else {
                return Integer.parseInt(howManyBombsString);
            }
        }
    }

    private static Area[][] pickLengthsOfArea(Scanner scanner) {
        String[] turnXandY;
        while(true) {
            System.out.println("Pick x.length and y.length of area(print \"x y\"): ");
            turnXandY = scanner.nextLine().split(" "); 
            if(turnXandY.length != 2) {
                System.out.println("print: \"x y\"!");
            }
            else if(!isNumeric(turnXandY[0]) || !isNumeric(turnXandY[1])) {
                System.out.println("x and y should be numbers!");
            }
            else if(Integer.parseInt(turnXandY[0]) <= 0 || Integer.parseInt(turnXandY[1]) <= 0) {
                System.out.println("x and y should be >0!");
            }
            else {
                return new Area[Integer.parseInt(turnXandY[0])][Integer.parseInt(turnXandY[1])];
            }
        }
    }

    private static boolean isXandYIn(int turnX,int turnY, Area[][] area) {
        if(turnX<0 || area[0].length<=turnX) {
            return false;
        }
        if(turnY<0 || area.length<=turnY) {
            return false;
        }
        return true;
    }

    public static boolean isNumeric(String strNum) {
        try {
            Integer.parseInt(strNum);
        } catch (NumberFormatException | NullPointerException nfe) {
            return false;
        }
        return true;
    }
}
class Area{
    private final ValueOfArea valueOfArea;

    private StatusOfArea statusOfArea;

    {
        statusOfArea = StatusOfArea.CLOSED;
    }
    Area(ValueOfArea valueOfArea){
        this.valueOfArea = valueOfArea;
    }
    ValueOfArea getValueOfArea() {
        return valueOfArea;
    }
    StatusOfArea getStatusOfArea() {
        return statusOfArea;
    }
    void setStatusOfArea(StatusOfArea statusOfArea) {
        this.statusOfArea = statusOfArea;
    }
}
class AreaWithXandY{
    private int x,y;
    Area area;

    AreaWithXandY(int x,int y,Area area){
        this.area = area;
        this.x = x;
        this.y = y;
    }
    int getX() {
        return x;
    }
    int getY() {
        return y;
    }
    Area getArea() {
        return area;
    }
    @Override
    public boolean equals(Object obj) {
        if(obj==null) {
            return false;
        }
        if(false==(obj instanceof AreaWithXandY)) {
            return false;
        }
        AreaWithXandY object = (AreaWithXandY) obj;
        if(object.getX()==x && object.getY()==y) { //its enouth
            return true;
        }
        else {
            return false;
        }
    }
}
enum StatusOfArea{
    MARKEDASBOMB,OPENED,CLOSED
}
enum ValueOfArea{
    BOMB("@"),
    NOONEBOMBAROUND("0"),ONEBOMBAROUND("1"),TWOBOMBAROUND("2"),TREEBOMBAROUND("3"),FOURBOMBAROUND("4"),FIVEBOMBAROUND("5"),SIXBOMBAROUND("6"),SEVENBOMBAROUND("7"),EIGHTBOMBAROUND("8");
    private String icon;
    ValueOfArea(String icon){
        this.icon = icon;
    }
    String getIcon() {
        return icon;
    }
    static ValueOfArea getValueOfAreaByString(String value) {
        ValueOfArea[] values = ValueOfArea.values();
        for(int i=0;i<values.length;i++) {
            if(values[i].getIcon().equals(value)) {
                return values[i];
            }
        }
        System.err.println("ERROR in getValueOfAreaByString(String value), we could not find this value");
        return null;
    }
}
\$\endgroup\$
  • \$\begingroup\$ To keep it short - your identifiers are way too long e.g. playerTurnsAndIsPlayersTurnInBomb which could also be split in two methods - one to check if user input was valid, the other to make the actual move. Also - you pass Area[][] around a lot. If you wrapped it in a class and exposed some methods the code would become a bit cleaner. It would also be easier to name the methods then. \$\endgroup\$ – jakubiszon Nov 1 '19 at 16:51
  • \$\begingroup\$ Please look around CodeReview or Goole for Java naming conventions and make sure you follow them. \$\endgroup\$ – TorbenPutkonen Nov 2 '19 at 7:19
2
\$\begingroup\$
  • When declaring variables, move the declaration towards the first usage. Moving declaration away from usage makes it harder to understand how your variables are actually being used.

  • When declaring variables, prefer final whenever possible. If you know that a value won't change, it's one less thing to keep track of as you're trying to read and understand code. For very short methods, this might not matter (since you can clearly see the whole method and therefore that it's not changed) but for long ones it's almost always better. This doesn't mean you need to declare them in ALL_CAPS. That's only appropriate for actual constants, not values that just happen to not be reassigned.

  • Use whitespace consistently and liberally around operators and punctuation. Every = should generally be surrounded by space, and every , should be followed by space. Similarly, you should have space inside if ( and while (. If possible, use an auto-formatter, which will handle this automatically and save you time.

  • Identifiers should be clear (avoiding the use of unnecessary abbreviations) but don't need to be as long as the ones you're using. This could be an indication that your functions are doing too much (meaning that they take on many unrelated responsibilities, so it's difficult to describe what they mean).

Apply the first and second points to Minesweeper.play, we get:

    public static void play() {
        final Scanner scanner = new Scanner(System.in);
        
        final String GREETING = "Hi, lets play minesweeper!";
        System.out.println(GREETING);
        final Area[][] area = pickLengthsOfArea(SCANNER);
        final int HOWMANYBOMBS = getHowManyBombs(area, scanner);
        System.out.println("hi");
        fillArea(area, HOWMANYBOMBS);

        final boolean isItWin;
        while (true) {
            printPole(area);
            if (playerTurnsAndIsPlayersTurnInBomb(area, scanner)) {
                isItWin = false;
                break;
            }
            if (weHaveNotGotAnyEmptyAreaThatNotCheckedAndWeMarkedOnlyBombs(area)) {
                isItWin = true;
                break;
            }
        }
        if (isItWin) {
            System.out.println("U won!");
        }
        else {
            System.out.println("Defieat!");
        }
    }
  • GREETING, area, isItWinshould befinal. scannershould be lowercase, since it's not a constant (it's just aScannerthat happens to not be reassigned) butGREETING` likely should be all-caps since it's actually a constant.

  • Variable declarations are moved down when possible (in particular, Area[][] area; area = ... is combined into final Area[][] area = ...)

    The while (true) loop to initialize `isItWin is a bit awkward, and is a good candidate for splitting into a new function (which is frequently the case when you find yourself needing to initialize a value inside a loop):

    private static playUntilWinOrLoss(Area[][] area, Scanner scanner) {
        while (true) {
            printPole(area);
            if (playerTurnsAndIsPlayersTurnInBomb(area, scanner)) {
                return false;
            }
            if (weHaveNotGotAnyEmptyAreaThatNotCheckedAndWeMarkedOnlyBombs(area)) {
                return true;
            }
        }
    }

Next we can just to printPole and take a look. We can see that there's some "symmetry" in the code, but it's not being exploited fully:

                if(area[y][x].getStatusOfArea()==StatusOfArea.CLOSED) {
                    System.out.print("?");
                }
                else if(area[y][x].getStatusOfArea()==StatusOfArea.OPENED) {
                    System.out.print(area[y][x].getValueOfArea().getIcon());
                }
                else if(area[y][x].getStatusOfArea()==StatusOfArea.MARKEDASBOMB) {
                    System.out.print("*");
                }

all three lines are of the form System.out.print( ... ), and they all inspect area[y][x] This suggests that the responsibility of identifying the appearance of the area should belong to the Area itself:

class Area {
    private final ValueOfArea value;
    private StatusOfArea status = StatusOfArea.CLOSED;

    Area(ValueOfArea value){
        this.value = value;
    }
    ValueOfArea getValue() {
        return value;
    }
    StatusOfArea getStatus() {
        return status;
    }
    void setStatus(StatusOfArea status) {
        this.status = status;
    }
    String getAppearance() {
        if (status == StatusOfArea.CLOSED) {
            return "?";
        }
        if (status == StatusOfArea.MARKEDASBOMB) {
            return "*";
        }
        return value.getIcon();
    }
}

Here we can make several improvements. First, adding _OfArea to the end of each name doesn't make anything clearer - they are all members of the Area class, so we already know that they belong to the Area. Making these names shorter allows us to scan faster without worrying about problems.

The status member variable can be initialized in one line as private StatusOfArea status = StatusOfArea.CLOSED;

The new getAppearance() method can be used by printPole:

    private static void printPole(Area[][] area) {
        System.out.println();
        for (int y = 0; y < area.length; y++) {
            for (int x = 0; x < area[0].length; x++) {
                System.out.print(area[y][x].getAppearance());
            }
            System.out.println();
        }
    }

Next, we can examine playerTurnsAndIsPlayersTurnInBomb.

  • Instead of using a chain of elses, we can instead use continue to retry the loop.

  • parseInt gets called 6 times when you only really need it twice: once for x and once for y (this is not about "efficiency"; it's about clarity: we're doing the same work over and over, so we could accidentally introduce an inconsistency between the call sites if any logic needed to be changed)

  • The name doesn't accurately describe what the function does. makeMove accurately described what it actually does; since the return value is vague we can make an enum for HIT or NOHIT:

The result is that we can remove entirely the function isXandYIn, and obtain the now-clearer:

    enum MoveHitsBomb {
        HIT, NOHIT
    }

    private static MoveHitsBomb makeMove(Area[][] area, Scanner scanner) {
        while(true) {
            System.out.println("Print \"open x y\", if u want open this area, print \"mark x y\", if u want mark this area as bomb or unmark this area");
            String[] commandAndXAndY = scanner.nextLine().split(" ");
            if (commandAndXAndY.length != 3) {
                System.out.println("fill out the form correctly!");
                continue;
            }

            final String action = commandAndXAndY[0];
            if (!action.equals("open") && !action.equal("mark")) {
                System.out.println("first work should be equal \"open\" or \"mark\"!");
                continue;
            }

            final int x;
            try {
                x = Integer.parseInt(commandAndXAndY[1]);
            } catch (NumberFormatException | NullPointerException nfe) {
                System.out.println("x should be a number!");
                continue;
            }

            final int y;
            try {
                y = Integer.parseInt(commandAndXAndY[2]);
            } catch (NumberFormatException | NullPointerException nfe) {
                System.out.println("y should be a number!");
                continue;
            }

            if (y < 0 || area.length <= y) {
                System.out.println("y should be between 0 and " + area.length-1);
                continue;
            }
            if (x < 0 || area[0].length <= x) {
                System.out.println("x should be between 0 and " + area[0].length-1);
                continue;
            }
        
            
            if (action.equals("open")) {
                if (area[y][x].getValue() == ValueOfArea.BOMB) {
                    return MoveHitsBomb.HIT;
                }
                
                area[y][x].setStatus(StatusOfArea.OPENED);
                if (area[y][x].getValue() == ValueOfArea.NOONEBOMBAROUND) {
                    openAllAround(x, y, area, new ArrayList<>());
                }
                return MoveHitsBomb.NOHIT;
            }
            else { // mark
                area[y][x].setStatusOfArea(StatusOfArea.MARKEDASBOMB);
                return MoveHitsBomb.NOHIT;
            }
        }

Next we might as well look at weHaveNotGotAnyEmptyAreaThatNotCheckedAndWeMarkedOnlyBombs. This function's name is describing what it does (which we can figure out by reading its code) but not what it means (which tells callers how to use it). isWinningBoard seems like a much more accurate name

  • cell.getValueOfArea() != ValueOfArea.BOMB /* is not a bomb */ is not a useful comment, since it's repeating what the code does. We can simplify to:
    private static boolean isWinningBoard(Area[][] area) {
        for (int y = 0; y < area.length; y++) {
            for (int x = 0; x < area[0].length; x++) {
                Area cell = area[y][x];
                if (cell.getValue() != ValueOfArea.BOMB && cell.getStatus() != StatusOfArea.OPENED) {
                    return false;
                }
            }
        }
        return true;
    }

It's less obvious where to go next to make minor cleanups, so I'm instead going to tackle AreaWithXandY: it probably shouldn't exist. It's clear why you're using it, but you just really don't actually want it to exist.

The main problem is your Area[][] array that you pass around everywhere. It's certainly useful, but it's not really pulling its weight. We can make a Board class that encapsulates the relevant information. This ends up having wide-reaching consequences for your program as a whole.

We don't actually need Area or AreaWithXandY. Once we have a Board class, we realize that we only really need StatusOfArea and ValueOfArea. We should also realize that we don't need any numbers in ValueOfArea; we just need EMPTY or BOMB. That's because the number that shows up is entirely presentational, and can be figured out by looking at the neighbors (from Board) rather than storing any additional info in the cell.

First, we need a position class:


final class Pos {
    public final int x;
    public final int y;
    Pos(int x, int y) {
        this.x = x;
        this.y = y;
    }
}

(if this was being used for something larger, I'd add an equals and hashCode implementation, but we don't need either for this small program). Note that we know the representation is definitely not changing (if it did, we'd have to rewrite a lot of stuff anyway) so it's fine to just make x and y into public final fields and skip the getters/setters.

We have the new enums

enum StatusOfArea {
    MARKEDASBOMB, OPENED, CLOSED
}
enum ValueOfArea {
    EMPTY, BOMB
}

and the Board:

class Board {
    private final ValueOfArea[][] values;
    private final StatusOfArea[][] statuses;

    public final int width;
    public final int height;

    Board(ValueOfArea[][] values) {
        this.values = values;
        this.height = values.length;
        this.width = values[0].length;
        this.statuses = new StatusOfArea[height][width];
        for (StatusOfArea[] row : statuses) {
            Arrays.fill(row, StatusOfArea.CLOSED);
        }
    }
    public String appearanceAt(Pos p) {
        if (statuses[p.y][p.x] == StatusOfArea.CLOSED) {
            return "?";
        }
        if (statuses[p.y][p.x] == StatusOfArea.MARKEDASBOMB) {
            return "*";
        }
        int count = countMineNeighbors(p);
        if (count == 0) {
            return "-";
        }
        return "" + count;
    }
    public boolean inBounds(Pos p) {
        return p.x >= 0 && p.x < width && p.y >= 0 && p.y < height;
    }
    public int countMineNeighbors(Pos p) {
        int count = 0;
        for (int dx = -1; dx <= 1; dx++) {
            for (int dy = -1; dy <= 1; dy++) {
                Pos neighbor = new Pos(p.x + dx, p.y + dy);
                if (this.inBounds(neighbor) && this.isBombAt(neighbor)) {
                    count++;
                }
            }
        }
        return count;
    }
    public boolean isBombAt(Pos p) {
        return this.values[p.y][p.x] == ValueOfArea.BOMB;
    }
    public boolean isOpenAt(Pos p) {
        return this.statuses[p.y][p.x] == StatusOfArea.OPENED;
    }

    public void markAsBombAt(Pos p) {
        this.statuses[p.y][p.x] = StatusOfArea.MARKEDASBOMB;
    }
    public void open(Pos p) {
        if (this.statuses[p.y][p.x] == StatusOfArea.OPENED) {
            return;
        }
        this.statuses[p.y][p.x] = StatusOfArea.OPENED;

        if (this.countMineNeighbors(p) == 0) {
            for (int dx = -1; dx <= 1; dx++) {
                for (int dy = -1; dy <= 1; dy++) {
                    Pos neighbor = new Pos(p.x + dx, p.y + dy);
                    if (this.inBounds(neighbor)) {
                        this.open(neighbor);
                    }
                }
            }
        }
    }
}

The Board constructor asks for the values to fill the grid with, and then it does the rest. In particular, it creates the statuses array and fills it with StatusOfArea.CLOSED.

Counting neighbors is important for the getAppearance() method, so that's provided as a convenience. It's also used inside the open() method, which is responsible for opening cells up and for performing the flood-fill (look how much shorter it is, now that it's in the right place, and our data reflects our usage!)

Everything else needs to be adjusted slightly (mostly to use Board methods instead of directly reading Area, which no longer exists) and to clean up the creation of the board.

The only one I really want to draw your attention to is fillAreaWithBombs, which uses an entirely different approach. In particular, it makes a list of locations and then shuffles them, filling the first howManyBombs items. This is much, much faster for large grids, because remove is linear (e.g. for a grid with 1,000,000 cells, this will be roughly 1,000,000 times faster).

    private static ValueOfArea[][] fillAreaWithBombs(int width, int height, int howManyBombs) {
        ValueOfArea[][] area = new ValueOfArea[width][height];
        ArrayList<Pos> listOfAllPositions = new ArrayList<>();
        for (int y = 0; y < area.length; y++) {
            for (int x = 0; x < area.length; x++) {
                area[y][x] = ValueOfArea.EMPTY;
                listOfAllPositions.add(new Pos(x, y));
            }
        }
        Collections.shuffle(listOfAllPositions);

        for (int i = 0; i < listOfAllPositions.size() && i < howManyBombs; i++) {
            area[listOfAllPositions.get(i).y][listOfAllPositions.get(i).x] = ValueOfArea.BOMB;
        }

        return area;
    }

Here's the whole program:

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;

import java.util.Scanner;

public class Minesweeper {
    public static void main(String[] args) {
        final Scanner scanner = new Scanner(System.in);
        
        final String GREETING = "Hi, lets play minesweeper!";
        System.out.println(GREETING);
        
        Pos corner = chooseSize(scanner);
        final int HOWMANYBOMBS = getHowManyBombs(corner.x, corner.y, scanner);
        
        final ValueOfArea[][] rawValues = fillAreaWithBombs(corner.x, corner.y, HOWMANYBOMBS);

        final boolean isItWin = playUntilWinOrLoss(new Board(rawValues), scanner);
        if (isItWin) {
            System.out.println("U won!");
        }
        else {
            System.out.println("Defieat!");
        }
    }

    private static boolean playUntilWinOrLoss(Board area, Scanner scanner) {
        while (true) {
            printPole(area);
            if (makeMove(area, scanner) == MoveHitsBomb.HIT) {
                return false;
            }
            if (isWinningBoard(area)) {
                return true;
            }
        }
    }

    private static boolean isWinningBoard(Board board) {
        for (int y = 0; y < board.height; y++) {
            for (int x = 0; x < board.width; x++) {
                if (!board.isBombAt(new Pos(x, y)) && !board.isOpenAt(new Pos(x, y))) {
                    return false;
                }
            }
        }
        return true;
    }


    enum MoveHitsBomb {
        HIT, NOHIT
    }

    private static MoveHitsBomb makeMove(Board area, Scanner scanner) {
        while(true) {
            System.out.println("Print \"open x y\", if u want open this area, print \"mark x y\", if u want mark this area as bomb or unmark this area");
            String[] commandAndXAndY = scanner.nextLine().split(" ");
            if (commandAndXAndY.length != 3) {
                System.out.println("fill out the form correctly!");
                continue;
            }

            final String action = commandAndXAndY[0];
            if (!action.equals("open") && !action.equals("mark")) {
                System.out.println("first work should be equal \"open\" or \"mark\"!");
                continue;
            }

            final int x;
            try {
                x = Integer.parseInt(commandAndXAndY[1]);
            } catch (NumberFormatException | NullPointerException nfe) {
                System.out.println("x should be a number!");
                continue;
            }

            final int y;
            try {
                y = Integer.parseInt(commandAndXAndY[2]);
            } catch (NumberFormatException | NullPointerException nfe) {
                System.out.println("y should be a number!");
                continue;
            }

            if (y < 0 || area.height <= y) {
                System.out.println("y should be between 0 and " + (area.height-1));
                continue;
            }
            if (x < 0 || area.width <= x) {
                System.out.println("x should be between 0 and " + (area.width-1));
                continue;
            }
        
            
            if (action.equals("open")) {
                if (area.isBombAt(new Pos(x, y))) {
                    return MoveHitsBomb.HIT;
                }
                
                area.open(new Pos(x, y));
                return MoveHitsBomb.NOHIT;
            }
            else { // mark
                area.markAsBombAt(new Pos(x, y));
                return MoveHitsBomb.NOHIT;
            }
        }
    }

    private static void printPole(Board area) {
        System.out.println();
        for (int y = 0; y < area.height; y++) {
            for (int x = 0; x < area.width; x++) {
                System.out.print(area.appearanceAt(new Pos(x, y)));
            }
            System.out.println();
        }
    }

    private static ValueOfArea[][] fillAreaWithBombs(int width, int height, int howManyBombs) {
        ValueOfArea[][] area = new ValueOfArea[width][height];
        ArrayList<Pos> listOfAllPositions = new ArrayList<>();
        for (int y = 0; y < area.length; y++) {
            for (int x = 0; x < area.length; x++) {
                area[y][x] = ValueOfArea.EMPTY;
                listOfAllPositions.add(new Pos(x, y));
            }
        }
        Collections.shuffle(listOfAllPositions);

        for (int i = 0; i < listOfAllPositions.size() && i < howManyBombs; i++) {
            area[listOfAllPositions.get(i).y][listOfAllPositions.get(i).x] = ValueOfArea.BOMB;
        }

        return area;
    }
    private static int getHowManyBombs(int width, int height, Scanner scanner) {
        while(true) {
            System.out.println("print number of bombs: ");
            String howManyBombsString = scanner.nextLine();
            if(!isNumeric(howManyBombsString)) {
                System.out.println("it should be number!");
            }
            else if(false==(0<Integer.parseInt(howManyBombsString) && Integer.parseInt(howManyBombsString) < width * height)) {
                System.out.println("it should be positive and it should not exceed the field capacity!");
            }
            else {
                return Integer.parseInt(howManyBombsString);
            }
        }
    }

    private static Pos chooseSize(Scanner scanner) {
        while (true) {
            System.out.println("Pick x.length and y.length of area(print \"x y\"): ");
            final String[] turnXandY = scanner.nextLine().split(" "); 
            if (turnXandY.length != 2) {
                System.out.println("print: \"x y\"!");
                continue;
            }
            if (!isNumeric(turnXandY[0]) || !isNumeric(turnXandY[1])) {
                System.out.println("x and y should be numbers!");
                continue;
            }
            if (Integer.parseInt(turnXandY[0]) <= 0 || Integer.parseInt(turnXandY[1]) <= 0) {
                System.out.println("x and y should be >0!");
                continue;
            }
            return new Pos(Integer.parseInt(turnXandY[0]), Integer.parseInt(turnXandY[1]));
        }
    }

    public static boolean isNumeric(String strNum) {
        try {
            Integer.parseInt(strNum);
        } catch (NumberFormatException | NullPointerException nfe) {
            return false;
        }
        return true;
    }
}

final class Pos {
    public final int x;
    public final int y;
    Pos(int x, int y) {
        this.x = x;
        this.y = y;
    }
}

enum StatusOfArea {
    MARKEDASBOMB, OPENED, CLOSED
}
enum ValueOfArea {
    EMPTY, BOMB
}

class Board {
    private final ValueOfArea[][] values;
    private final StatusOfArea[][] statuses;

    public final int width;
    public final int height;

    Board(ValueOfArea[][] values) {
        this.values = values;
        this.height = values.length;
        this.width = values[0].length;
        this.statuses = new StatusOfArea[height][width];
        for (StatusOfArea[] row : statuses) {
            Arrays.fill(row, StatusOfArea.CLOSED);
        }
    }
    public String appearanceAt(Pos p) {
        if (statuses[p.y][p.x] == StatusOfArea.CLOSED) {
            return "?";
        }
        if (statuses[p.y][p.x] == StatusOfArea.MARKEDASBOMB) {
            return "*";
        }
        int count = countMineNeighbors(p);
        if (count == 0) {
            return "-";
        }
        return "" + count;
    }
    public boolean inBounds(Pos p) {
        return p.x >= 0 && p.x < width && p.y >= 0 && p.y < height;
    }
    public int countMineNeighbors(Pos p) {
        int count = 0;
        for (int dx = -1; dx <= 1; dx++) {
            for (int dy = -1; dy <= 1; dy++) {
                Pos neighbor = new Pos(p.x + dx, p.y + dy);
                if (this.inBounds(neighbor) && this.isBombAt(neighbor)) {
                    count++;
                }
            }
        }
        return count;
    }
    public boolean isBombAt(Pos p) {
        return this.values[p.y][p.x] == ValueOfArea.BOMB;
    }
    public boolean isOpenAt(Pos p) {
        return this.statuses[p.y][p.x] == StatusOfArea.OPENED;
    }

    public void markAsBombAt(Pos p) {
        this.statuses[p.y][p.x] = StatusOfArea.MARKEDASBOMB;
    }
    public void open(Pos p) {
        if (this.statuses[p.y][p.x] == StatusOfArea.OPENED) {
            return;
        }
        this.statuses[p.y][p.x] = StatusOfArea.OPENED;

        if (this.countMineNeighbors(p) == 0) {
            for (int dx = -1; dx <= 1; dx++) {
                for (int dy = -1; dy <= 1; dy++) {
                    Pos neighbor = new Pos(p.x + dx, p.y + dy);
                    if (this.inBounds(neighbor)) {
                        this.open(neighbor);
                    }
                }
            }
        }
    }
}
```
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Great analysis! But I can’t understand why we need to constantly count how many bombs are around? This value is constant for each field, and its score is very expensive. I would like to make a good structure, but bombs should not have this element. I can not imagine the right abstraction for this. \$\endgroup\$ – Miron Nov 3 '19 at 14:32
  • \$\begingroup\$ Why don't we show the first variables that will be used in the program? Wouldn't that improve code readability? \$\endgroup\$ – Miron Nov 3 '19 at 15:33
  • \$\begingroup\$ The runtime cost of computing bomb-count numbers on-the-fly is very low (only 9 array accesses and some increments!) so it's unlikely to have a large impact on program performance (and you'd need to benchmark and measure before assuming that it's actually a problem). \$\endgroup\$ – Nathan Nov 3 '19 at 19:18
  • \$\begingroup\$ I'm not exactly sure what you mean by your second comment. When you're reading a block of code, you're expecting to learn the "how" by just looking at the code. Well-structured code makes the "why" clear. If you put all your variables at the top of the function, the reader has no idea why they're there until they're used in context. Then when they're reading the code, they have to remind themselves whether this is the first assignment, or a reassignment, or whatever, instead of just knowing that "oh, at this point it makes sense to give this value a name". \$\endgroup\$ – Nathan Nov 3 '19 at 19:21
  • \$\begingroup\$ Java SE, Documentation: "6.3 Placement Put declarations only at the beginning of blocks. (A block is any code surrounded by curly braces "{" and "}".) Don't wait to declare variables until their first use; it can confuse the unwary programmer and hamper code portability within the scope." \$\endgroup\$ – Miron Nov 4 '19 at 5:38
2
\$\begingroup\$

Afterthought / tldr;

If you start naming your methods & variables descriptively, using the suggestions below & finding more information/examples online it will be a huge 'level up' in your programming. Everything else will become easier.

General coding standards

Use static final variables for messages. Alternatively these could be taken from a properties file

Variable names that are multiple words are always separated somehow. With static final variables are all caps separated by a _. Class names are UpperCamelCase & variables are lowerCamelCase.

Declare variables when they are used. As locally scoped as possible

Separate arguments/parameters with a space

Use the condition inside the while/do while loop. Try to avoid while(true).

Use spaces before/after =.

Don't use magic Strings/numbers

Use a white space to separate conditionals

Use a line of white space to separate methods

Don't do this:

private StatusOfArea statusOfArea;
{
    statusOfArea = StatusOfArea.CLOSED;
}

Each class/enum should be in it's own Java file.

Command (Open / Mark) should be it's own ENUM or class.

Take advantage of the toString() method. Also keep logic contained in the proper class/enum. You should be printing the grid by calling the toString() on each object. Note that System.out.print will invoke the toString method.

You can compare Strings without worrying about case by using the String method equalsIgnoreCase.

I am not sure what printPole means but it looks like it's printing out the grid. Even if you want the icon representing the item and what's printed out to be different, the logic/symbols should be contained in the Enum itself. For example, by having another property on the ENUM.

Separate unrelated logic into a separate class. For example, Minesweeeper should not be responsible for checking if a String is numeric. I mentioned this in the other CR linked.

Although it's kind of cheating, (doesn't technically follow best practices (single responsibility)) An easy way to do this is to have a Helper class or a Util class. This class can include all of your functions such as checking if a String is numeric. It would not validating a Move for example, though. That would either be in a MoveValidation class, or the Minesweeper class. If you are confused as what can go in this Utility class, leave it for last and put all "Other" methods in it. (Methods that you can't find a class for or justify creating a class for)

It's better to be converting a String all in one place. For example maybe one day you have troubles with certain input being formatted wrong or certain numbers are throwing an error. It's much easier if you have a single method.

It's also good when the method changes often. An example is if you've had to change methods often, such as moving away from apache.utils.String. Your class would be a simple wrapper for some or all of the functions.

Never put == true or == false in a condition. It's redundant. Use ! for false.

Don't run methods multiple times when you don't need to. For example in your if statement you're parsing the input as int twice.

Printing an error and returning null could lead to a NullPointerException or other issues. It's possible you may just look at the last error & not see your error message. I suggest throwing an Exception instead. You could throw a RuntimeException if you want.

Here we want the numberOfBombs selected by the user to be less than the total area. It's not crystal clear though. You could make a variable named totalSpacedOnGrid, or you could create a method called getTotalNumberOfSpaces and input area as a paramater. However I think a comment is best in this situation.

(area.length * area[0].length) >= numberOfBombsSelectedByUser

Users prefer reading "greater than" rather than ">" in messages that the user sees.

Declare variables as locally as possible.

Take advantage of String.format. Lots to be said here, information available online.

Java Streams

Lots of cases here are good candidates for using Streams if you're using Java 8+

Naming variables

Avoid "I" or "we" in naming variables/methods.

Coordinate is a better name than AreaWithXAndY. The alternative to this class is an array of 2 ints. (int[2])

isItWin should be renamed isWin.

Avoid double negatives. weHaveNotGotAnyEmptyAreaThatNotCheckedAndWeMarkedOnlyBombs is an awkward read. If you are checking if the player has won or not, you can name the method playerHasWon then use a JavaDoc to explain the conditions.

howManyBombs reads like a question. It should not be a variable name. A better one would be numberOfBombs. If you'd like to be explicit that it was chosen from the user, numberOfBombsSelectedByUser. However note your naming should only relate to the scope it's in, not the whole class. So in your fillAreaWithBomb method, we don't care if the parameter came from the user or not. So don't name it in relation to where it came from.

HOW_MANY_BOMBS is not a static constant, so it should be named howManyBombs. (But changed again as I mentioned above). For more info on naming conventions for 'final but not static' see here.

Area is not a good class name. It only represents one spot in the total area. Tbh I cannot think of a good name. It's a "Square" or a "Mark" I guess.

playerTurnsAndIsPlayersTurnInBomb

playerTurnsAndIsPlayersTurnInBomb is not correct. By the name it suggests it'll return false if it's not the players turn (but it's always the players turn since theirs only 1 player). This method should also be broken up.

This method should not be determining a loss. It's doing way too much. Try to think in terms of the method, rather than the whole class.

Use Objects, don't list all methods as static

A common beginner mistake is to make all methods static since you get a warning when calling them in the main method. This is a bad habit, you should stop it now.

The best way to break out of bad habits is to never learn them (You can't unlearn things but just wanted to mention you've learned the wrong way to get around that warning). Instead you should be calling the methods on an instance of a Minesweeper object

Don't over complicate logic

Methods should be doing 1 thing. Validation could be a method (I mentioned this in the other Codereview).

Never do false == 0 <.. or !0 < .... Instead use the >= (greater than or equal to) symbol.

Don't ignore warnings

When you see depreciated warnings assuming you are using an IDE you should go into the method and see what alternative method you should be using. If you're not using an IDE try googling the method to see a non-depreciated alternative.

Don't repeat yourself

Your fillAreaWithBombs and fillAreaWithEmptyArea could be refactored to reduce repeating yourself. What if you had a method that filled an area with the value passed as a parameter. You could also have a boolean to only set if the field is null.

Either set all fields to 'empty' then add bombs (no checking for null fields), or set appropriate bombs then add the null fields to 'empty' (with checking to ensure you don't overwrite bombs).

A third option is to use null as your EMPTY. Then just fill bombs.

Main logic is overly complicated

Your area classes are overly complicated. You should just have an ENUM which states what a space could be, then have a 2d array of that enum.

If you want the mark/spot to have an X/Y that could be a property of the ENUM, or you could make it a class instead of an ENUM (As you have now). However I don't see it being necessary. Since it's inside a 2d array, you need the X/Y to access it anyway. So you'll never need to get it from the spot.

No need to use an ArrayList for filling the 2D array. Whenever you write this kind of logic (In this case, getting X random numbers between y-z without duplicates), it should be placed into it's own method and be documented or clearly written. Unless you are sure it's the 'best way' of doing it, chances are it could be refactored later. It's also important for other developers reading it to understand it.

I don't know what getAllAreasAround is doing exactly but you don't need it. It's overly complicated. Same for getAllAreasAroundWithTheirXandY.

Suggestions for additional features

Don't let the player select the same place twice

Count how many turns the player has made. Alternatively you could count based on what the board looks like at the end

Add a debug option to let you see the bombs on the grid. For example, if an argument is given on the command line to 'debug'.

Add a timer & 'number of bombs not flagged' counter just like Microsoft minesweeper.

Show the full grid when the player loses/wins.

Modified code:

package Test;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Scanner;
import static Test.Utility.isNumeric;
import static Test.Utility.convertSingleDigitIntegerToCharacter;
import static Test.Utility.getRandomCoordinatesWithoutDuplicates;

public class Minesweeper {
    private static final String GREETING_MESSAGE = "Hi, lets play minesweeper!";
    private static final String PICK_X_AND_Y_AREA_MESSAGE = "Pick x.length and y.length of area(print \"x y\"): ";
    private static final String NUMBER_OF_BOMBS_PROMPT = "print number of bombs: ";
    private static final String PROMPT_USER_FOR_COMMAND = "Print \"open x y\", if u want open this area, print \"mark x y\", if u want mark this area as bomb or unmark this area";

    private static final String INCORRECT_COMMAND_NUMBER_OF_ARGUMENTS = "fill out the form correctly!";
    private static final String INVALID_COMMAND = "first work should be equal \"open\" or \"mark\"!";
    private static final String COMMAND_NOT_A_NUMBER = "x and y should be numbers!";
    private static final String INVALID_COORDINATE = "x and y should be in area! P.S.: area.lengthY=%s. area.lengthX=%s";

    private static final Scanner SCANNER = new Scanner(System.in);

    public void play() {
        boolean isWin = false;

        System.out.println(GREETING_MESSAGE);
        Mark[][] grid = getLengthsOfAreaFromUser(SCANNER);

        final int numberOfBombs = getNumberOfBombsFromUser(grid, SCANNER);
        System.out.println("hi");
        fillArea(grid, numberOfBombs);

        while(true) {
            printGrid(grid, true);

            Mark mark = playerMakeAMove(grid, SCANNER);

            if(mark == Mark.BOMB) {
                isWin = false;
                break;
            }
            else if(playerHasWonGame(grid)) {
                isWin = true;
                break;
            }
        }

        if(isWin) {
            printGrid(grid, false);
            System.out.println("U won!");
        }
        else {
            printGrid(grid, false);
            System.out.println("Defieat!");
        }
    }

    private boolean playerHasWonGame(Mark[][] area) {
        for (Mark[] markArray : area) {
            for (Mark mark : markArray) {
                if (mark == Mark.EMPTY) {
                    return false;
                }
            }
        }

        return true;
    }

    /**
     * Prompt the user to open a location or mark a location & return the updated {@link Mark} at the location 
     */
    private Mark playerMakeAMove(Mark[][] area, Scanner scanner) {
        CommandSelected selectedCommand = getValidCommandFromUser(scanner, area);
        int x = selectedCommand.getCoordinate().getX();
        int y = selectedCommand.getCoordinate().getY();

        Mark markOnArea = area[y][x];
        if(selectedCommand.getUserOption() == UserOption.OPEN) {
            if(markOnArea == Mark.BOMB) {
                return Mark.BOMB;
            }
            else {
                int numberOfBombsSurrounding = countBombsAtLocations(area, getSurroundingCoordinates(area, false, selectedCommand.getCoordinate()));

                if (numberOfBombsSurrounding == 0) {
                    area[y][x] = Mark.OPEN;
                    openAllAround(x, y, area);
                }
                else {
                    // E.G convert from 1 to "Mark.ONE_BOMB".
                    char character = convertSingleDigitIntegerToCharacter(numberOfBombsSurrounding);
                    area[y][x] = Mark.valueOf(character);
                }

                return area[y][x];
            }
        }
        else {
            area[y][x] = Mark.MARKED_AS_BOMB;
            return Mark.MARKED_AS_BOMB;
        }
    }

    private CommandSelected getValidCommandFromUser(Scanner scanner, Mark[][] area) {
        while (true) {
            System.out.println(PROMPT_USER_FOR_COMMAND);
            String[] commandAndXAndY = scanner.nextLine().split(" ");

            if(commandAndXAndY.length != 3) {
                System.out.println(INCORRECT_COMMAND_NUMBER_OF_ARGUMENTS);
            }
            else {
                String commandSelected = commandAndXAndY[0];
                String xSelected = commandAndXAndY[1];
                String ySelected = commandAndXAndY[2];

                if(!commandSelected.equalsIgnoreCase("open") && commandSelected.equalsIgnoreCase("mark")) {
                    System.out.println(INVALID_COMMAND);
                }
                else if(!isNumeric(xSelected) || !isNumeric(ySelected)) {
                    System.out.println(COMMAND_NOT_A_NUMBER);
                }
                else if(!isValidCoordinate(Integer.parseInt(xSelected), Integer.parseInt(ySelected), area)) {
                    System.out.print(String.format(INVALID_COORDINATE, area.length, area[0].length));
                }
                else {
                    return new CommandSelected(UserOption.fromValue(commandSelected), new Coordinate(Integer.parseInt(xSelected), Integer.parseInt(ySelected)));
                }
            }
        }
    }

    private void openAllAround(int x, int y, Mark[][] area) {
        List<Coordinate> coordinatesInArea = getAllSurroundingCoordinates(area, false, x, y);

        for(Coordinate coordinate : coordinatesInArea) {
            Mark markAtLocation = area[coordinate.getY()][coordinate.getX()];

            if (!hasLocationBeenMarked(markAtLocation) && Mark.BOMB != markAtLocation) {
                int numberOfBombsSurrounding = countBombsAtLocations(area, getSurroundingCoordinates(area, false, coordinate));

                if (numberOfBombsSurrounding == 0) {
                    area[coordinate.getY()][coordinate.getX()] = Mark.OPEN;
                    openAllAround(coordinate.getX(), coordinate.getY(), area);
                }
                else {
                    // E.G convert from 1 to "Mark.ONE_BOMB".
                    char character = convertSingleDigitIntegerToCharacter(numberOfBombsSurrounding);
                    area[coordinate.getY()][coordinate.getX()] = Mark.valueOf(character);
                }
            }
        }
    }

    private boolean hasLocationBeenMarked(Mark location) {
        final List<Mark> LOCATIONS_THAT_SHOULD_NOT_BE_MARKED = Arrays.asList(
                Mark.MARKED_AS_BOMB,
                Mark.ONE_BOMB_AROUND,
                Mark.TWO_BOMB_AROUND,
                Mark.THREE_BOMB_AROUND,
                Mark.FOUR_BOMB_AROUND,
                Mark.FIVE_BOMB_AROUND,
                Mark.SIX_BOMB_AROUND,
                Mark.SEVEN_BOMB_AROUND,
                Mark.EIGHT_BOMB_AROUND,
                Mark.OPEN
        );

        return LOCATIONS_THAT_SHOULD_NOT_BE_MARKED.contains(location);
    }

    /**
     * Returns all valid unmarked coordinates surrounding the area of the coordinate passed
     */
    private List<Coordinate> getAllSurroundingCoordinates(Mark[][] area, boolean includeCoordinatePassed, int xCoord, int yCoord) {
        List<Coordinate> validCoordinatesInArea = new ArrayList<Coordinate>();

        // We want the coord itself, the coord -1 and the coord + 1 to get the full area
        for (int x = xCoord - 1; x <= xCoord + 1; x++) {
            for (int y = yCoord - 1; y <= yCoord + 1; y++) {
                // Skip this coordinate if appropriate
                if (includeCoordinatePassed || xCoord != x || yCoord != y) {
                    if (isValidCoordinate(x, y, area)) {
                        validCoordinatesInArea.add(new Coordinate(x, y));
                    }
                }
            }
        }

        return validCoordinatesInArea;
    }

    private List<Coordinate> getSurroundingCoordinates(Mark[][] area, boolean includeCoordinatePassed, Coordinate coordinate) {
        return getAllSurroundingCoordinates(area, includeCoordinatePassed, coordinate.getX(), coordinate.getY());
    }

    private int countBombsAtLocations(Mark[][] grid, Coordinate... coordinatesToCountBombsIn) {
        int numberOfBombs = 0;

        for (Coordinate coordinate : coordinatesToCountBombsIn) {
            if (grid[coordinate.getY()][coordinate.getX()] == Mark.BOMB) {
                numberOfBombs++;
            }
        }

        return numberOfBombs;
    }

    private int countBombsAtLocations(Mark[][] grid, List<Coordinate> coordinatesToCountBombsIn) {
        Coordinate[] coordinateList = new Coordinate[coordinatesToCountBombsIn.size()];
        return countBombsAtLocations(grid, coordinatesToCountBombsIn.toArray(coordinateList));
    }

    private static void printGrid(Mark[][] grid, boolean hideBombs) {
        System.out.println();
        for(int y = 0; y < grid.length; y++) {
            for(int x=0; x < grid[0].length; x++) {
                if (hideBombs && grid[y][x] == Mark.BOMB) {
                    System.out.print(Mark.EMPTY);
                }
                else {
                    System.out.print(grid[y][x]);
                }

            }
            System.out.println();
        }
    }

    private static void fillArea(Mark[][] area, int numberOfBombs) {
        fillArea(area, Mark.EMPTY);

        // fill area with bombs
        fillAreaWithRandomMarks(area, numberOfBombs, Mark.BOMB);
    }

    private static void fillArea(Mark[][] area, Mark mark) {
        for (int y = 0; y < area.length; y++) {
            for (int x = 0; x < area[y].length; x++) {
                area[x][y] = mark;
            }
        }
    }

    private static void fillAreaWithRandomMarks(Mark[][] area, int numberOfBombs, Mark mark) {
        int[][] bombLocations = getRandomCoordinatesWithoutDuplicates(numberOfBombs, area.length, area[0].length);

        for (int i = 0; i < bombLocations.length; i ++) {
            int x = bombLocations[i][0];
            int y = bombLocations[i][1];

            area[x][y] = mark;
        }
    }

    private int getNumberOfBombsFromUser(Object[][] area, Scanner scanner) {
        int numberOfBombs = -1;
        boolean isValidNumberOfBombs = false;

        do
        {
            numberOfBombs = getValidNumberFromUser(scanner, NUMBER_OF_BOMBS_PROMPT);
            isValidNumberOfBombs = isValidNumerOfBombs(numberOfBombs, area);

            if (!isValidNumberOfBombs) {
                System.out.println("it should be positive and it should not exceed the field capacity!");
            }
        } while (!isValidNumberOfBombs);

        return numberOfBombs;
    }

    private int getValidNumberFromUser(Scanner scanner, String prompt) {
        System.out.println(prompt);
        String inputFromUser = scanner.nextLine();

        while (inputFromUser == null || !isNumeric(inputFromUser)) {
            System.out.println("it should be number!");
            inputFromUser = scanner.nextLine();
        }

        return Integer.parseInt(inputFromUser);
    }

    private boolean isValidNumerOfBombs(int numberOfBombs, Object[][] area) {
        // Between 1 and the max number of spaces
        return numberOfBombs > 0 && (area.length * area[0].length) >= numberOfBombs;
    }

    private static Mark[][] getLengthsOfAreaFromUser(Scanner scanner) {
        Mark[][] gridCreated = null;
        while(gridCreated == null) {
            System.out.println(PICK_X_AND_Y_AREA_MESSAGE);
            String[] turnXandY = scanner.nextLine().split(" "); 

            if(turnXandY.length != 2) {
                System.out.println("print: \"x y\"!");
            }
            else if(!isNumeric(turnXandY[0]) || !isNumeric(turnXandY[1])) {
                System.out.println("x and y should be numbers!");
            }
            else if(Integer.parseInt(turnXandY[0]) <= 0 || Integer.parseInt(turnXandY[1]) <= 0) {
                System.out.println("x and y should be greater than 0!");
            }
            else {
                gridCreated = new Mark[Integer.parseInt(turnXandY[0])][Integer.parseInt(turnXandY[1])];
            }
        }

        return gridCreated;
    }

    private static boolean isValidCoordinate(int x, int y, Mark[][] area) {
        if(x < 0 || area[0].length <= x) {
            return false;
        }
        if(y < 0 || area.length <= y) {
            return false;
        }
        return true;
    }
}

class Coordinate {
    int x, y;

    public Coordinate(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() {
        return this.x;
    }

    public int getY() {
        return this.y;
    }

    public int[] toArray() {
        return new int[]{x, y};
    }
}

enum UserOption {
    MARK("MARK"),
    OPEN("OPEN");

    private String option;

    UserOption(String option) {
        this.option = option;
    }

    public static UserOption fromValue(String option) {
        for (UserOption userOption : values()) {
            if (userOption.option.equalsIgnoreCase(option)) {
                return userOption;
            }
        }

        return null;
    }
}

class CommandSelected {
    private UserOption optionSelected;
    private Coordinate coordinateSelected;

    public UserOption getUserOption() {
        return this.optionSelected;
    }

    public Coordinate getCoordinate() {
        return this.coordinateSelected;
    }

    public CommandSelected(UserOption option, Coordinate coordinate) {
        this.optionSelected = option;
        this.coordinateSelected = coordinate;
    }
}

class Utility {
    public static boolean isNumeric(String input) {
        try {
            Integer.parseInt(input);
        } catch (NumberFormatException | NullPointerException nfe) {
            return false;
        }
        return true;
    }

    public static int[][] getRandomCoordinatesWithoutDuplicates(int numberOfCoordinates, int widthOfGrid, int heightOfGrid) {
        List<Coordinate> cordinates = new ArrayList<Coordinate>();
        int[][] randomCoordinates = new int[numberOfCoordinates][2];

        // Get a list of all possible coordinates
        for (int x = 0; x < widthOfGrid; x++) {
            for (int y = 0; y < heightOfGrid; y++) {
                cordinates.add(new Coordinate(x, y));
            }
        }

        // shuffle aka randomize the list
        Collections.shuffle(cordinates);

        // Take top X
        for (int i = 0; i < numberOfCoordinates; i++) {
            randomCoordinates[i] = cordinates.get(i).toArray();
        }

        return randomCoordinates;
    }

    public static char convertSingleDigitIntegerToCharacter(int number) {
        return String.valueOf(number).charAt(0);
    }
}

// A placement can either be a Bomb, marked by user, selected by user, or "empty" aka untouched & not a bomb
enum Mark{
    BOMB('@'), 
    MARKED_AS_BOMB('X'), 
    EMPTY('_'), 
    OPEN('W'),
    ONE_BOMB_AROUND('1'),
    TWO_BOMB_AROUND('2'),
    THREE_BOMB_AROUND('3'),
    FOUR_BOMB_AROUND('4'),
    FIVE_BOMB_AROUND('5'),
    SIX_BOMB_AROUND('6'),
    SEVEN_BOMB_AROUND('7'),
    EIGHT_BOMB_AROUND('8');

    char character;

    Mark(char character) {
        this.character = character;
    }

    public static Mark valueOf(char character) {
        for (Mark mark : values()) {
            if (character == mark.character) {
                return mark;
            }
        }

        return null;
    }

    @Override
    public String toString() {
        return String.valueOf(character);
    }
}
| improve this answer | |
\$\endgroup\$

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.