7
\$\begingroup\$

Am trying to create a minesweeper game, I have the following code which has to many nested ifs, I want to change the code to take out the nested ifs and make it better to read. But I am stuck on how to change the nested ifs. Any help would be great. thanks

   /*method to check cells around a cell and set its value.*/
   public void setCellValues(){
            for(int i = 0; i<side; i++){
                for(int j = 0; j<side; j++){
                     if(cells[i][j].getValue() != -1){
                         if(j>=1 && cells[i][j-1].getValue() == -1) cells[i][j].incrementValue();
                         if(j<= limit && cells[i][j+1].getValue() == -1) cells[i][j].incrementValue();
                         if(i>=1 && cells[i-1][j].getValue() == -1) cells[i][j].incrementValue();
                         if(i<= limit && cells[i+1][j].getValue() == -1) cells[i][j].incrementValue();
                         if(i>=1 && j>= 1 && cells[i-1][j-1].getValue() == -1) cells[i][j].incrementValue();
                         if(i<= limit && j<= limit && cells[i+1][j+1].getValue() == -1) cells[i][j].incrementValue();
                         if(i>=1 && j<= limit && cells[i-1][j+1].getValue() == -1) cells[i][j].incrementValue();
                         if(i<= limit && j>= 1 && cells[i+1][j-1].getValue() == -1) cells[i][j].incrementValue();
                     }
                }
            }
        }
        /*This method starts chain reaction. When user click on particular cell, if cell is empty (value = 0) this
        method look for other empty cells next to activated one. If finds one, it call checkCell and in effect,
        start next scan on its closest area.
         */
        public void scanForEmptyCells(){
            for(int i = 0; i<side; i++){
                for(int j = 0; j<side; j++){
                    if(!cells[i][j].isNotChecked()){
                        if(j>=1 && cells[i][j-1].isEmpty()) cells[i][j-1].checkCell();
                        if(j<= limit && cells[i][j+1].isEmpty()) cells[i][j+1].checkCell();
                        if(i>=1 && cells[i-1][j].isEmpty()) cells[i-1][j].checkCell();
                        if(i<= limit && cells[i+1][j].isEmpty()) cells[i+1][j].checkCell();
                        if(i>=1 && j>= 1 && cells[i-1][j-1].isEmpty()) cells[i-1][j-1].checkCell();
                        if(i<= limit && j<= limit && cells[i+1][j+1].isEmpty()) cells[i+1][j+1].checkCell();
                        if(i>=1 && j<= limit && cells[i-1][j+1].isEmpty()) cells[i-1][j+1].checkCell();
                        if(i<= limit && j>= 1 && cells[i+1][j-1].isEmpty()) cells[i+1][j-1].checkCell();
                    }
                }
            }
        }
\$\endgroup\$
  • \$\begingroup\$ Please don't remove the code from your question as it invalidates answers and makes the question unusable for other users of the site. If there's a reason you'd like to remove the question, let me know and I'll see if we can solve the problem. \$\endgroup\$ – Pimgd Apr 9 '16 at 12:26
1
\$\begingroup\$

Procedural way

I usually suggest doing it in a way such that you don't have to perform to many checks. Also, I think that iterating on cells and updating the contour when you find a mine is logically simpler than detecting adjacent mines for each cell.

I will not write code for you (you are trying to learn something in the end), but I will outline the overall algorithm, and offer an object-oriented approach to the problem - which has smaller checks than your current solution and is substantially more readable.

Phase 1: corners

Write, for each corner, a single if statement. If it is a mine, update the three surrounding cells if they are not mines.

Phase 2: borders (- corners)

Iterate over the four borders (with 4 for loops), updating the 5 adjacent cells if they are not mines.

Phase 3 (final): the center of the board

For each cell in the center, if it is a mine, update all 8 surrounding cells if they are not mines.

Object-oriented way

Let every cell be an instance of a class Cell¹. A Cell knows (and exposes) whether it is a mine or not. It also has a public method incrementNeighborCounter()².

Done.

Really, we're done. (Yes, you need to implement Cell, but the essential part of programming [designing] has been done already, and you seem capable of writing the code of Cell).

How does it work? You go through the board once, using the pattern described in the "Procedural way" above. But you don't check if a cell is a mine or not before telling it to increment its counter (because that has been delegated to how a Cell presents itself [mines can even count their neighboring mines too now]), you just call incrementNeighborCounter() and move ahead. Part of the logic is now in Cell and your code is simpler, lighter on the brain and less likely to contain bugs.

¹ - It could be an Interface or an Abstract Class, just don't go overboard and add unnecessary complexity.

² - Name it whatever you feel like, this is not an exact science. Just try to be descriptive and use the name as documentation.


Just in case you are wondering, a very simple Cell implementation could use a boolean for being a mine or not and an integer for counting how many mine neighbors it has.

\$\endgroup\$
  • 1
    \$\begingroup\$ Just manually write the cases that would pass for it. For instance, for the top left corner, you can get (0, 1), (1, 0), and (1, 1). Each corner has three adjacent cells, just index them based on their offsets from the corner cell. The bottom right, as another example, would get (x_max - 1, y_max - 1), (x_max - 1, y_max), (x_max, y_max - 1). \$\endgroup\$ – Bernardo Sulzbach Apr 6 '16 at 1:23

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.