I'm still new to Java and learning programming, so my main goal is to learn to making good code, so I don't want to do anything complex until I fix all of my bad coding habits on smaller programs.So, this is my third version of this program, you can find last one here and here is where I started. I wanted to expand the game with few extra features. First one is that it now has interface, also, player can get points when he plays and base on how much points he has, he can play different levels of the game.Also, username and points are saved to text file, first I wanted to use same text file for that but updating points in that looks quite complicated so I thought this is better way since I only have this to informations I want to store, so maybe I'll do that in next version if it's better or maybe there is better way of storing/updating points?I'm not sure if I'm using communication between two classes (player and game) as I should, it's something I should look more into.  Main.javaimport java.io.IOException;public class Main {    public static void main(String[] args) throws IOException {        Game game = new Game();        Player player = new Player();        game.options(player);    }}  Game.javaimport java.io.IOException;import java.util.concurrent.ThreadLocalRandom;import javax.swing.JOptionPane;public class Game {    private int min=1;    private int max;    private int numberOfMines;    private int randomNumber;    int mine[] = new int[20];    private int userGuess;    int option = 0;    public int promptForInteger(String messageText) {        String askBoxes = JOptionPane.showInputDialog(messageText);        int number = 0;        if(askBoxes == null) {            System.exit(0);        } else {                try {                    number = Integer.parseInt(askBoxes);                } catch(NumberFormatException e) {                   JOptionPane.showMessageDialog(null, Value must be number!);                }        }        return number;    }        public void options(Player player) throws IOException{        do{            option = promptForInteger(Hi  + player.name + ! \n                    + Please select your game:\n1) Guess number between 1 and 10 with 1 mine.\n                    + 2) Guess number between 1 and 20 with 3 mines.  (You need 20 points to play this)\n                    + 3) Guess number between 1 and 100 with 20 mines.  (You need 100 points to play this)\n                    + 4) Exit the game (no points needed for this :) )                    + \n \n \n you currently have:  + player.points +  points \n \n);        // option = promptForInteger();        }while (option < 1 || option > 4);        switch (option) { // set max depending on user selection            case 1: max = 10;                    numberOfMines = 0; // one mine                    break;            case 2: if (player.points < 10){                        JOptionPane.showMessageDialog(null, You don't have enough points to play this game);                        options(player);                        return;                    }                    max = 20;                    numberOfMines = 2; // 3 mines                    break;            case 3: if (player.points < 100){                        JOptionPane.showMessageDialog(null, You don't have enough points to play this game);                        options(player);                        return;                    }                    max = 100;                    numberOfMines = 19; // 20 mines                    break;              case 4: System.exit(0);        }                createNumberAndMines();        playGame(player);    }    public void createNumberAndMines(){          randomNumber = getRandom(); // get random number to guess         askForGuess(); // first user guess can't be mine, so I want to ask for first guess then set mines         for (int i = numberOfMines; i >= 0; i--){             do{ // we don't want to have same number for guessing and mine or first user guess to be a mine                 mine[i] = getRandom();                 for (int j = i+1; j<19; j++){ //check if generated number is duplicate mine                     if (mine[i] == mine[j]){                         i++; // if we find that this number is duplicated to previous one we want to do for loop again for this mine[i]                      }                 }             }while (mine[i] == randomNumber || mine[i] == userGuess);         }    }    public int getRandom() {        return ThreadLocalRandom.current().nextInt(min, max + 1);    }    public void playGame(Player player) throws IOException {        for (int numGuesses = 0; true; ){            if (userGuess > 0) numGuesses++; // if user inputs something other then number we don't want to increase numGuesses            for (int i=numberOfMines; i>=0; i--){// check if user hits mine                if (mine[i] == userGuess){                    JOptionPane.showMessageDialog(null, You lose! You have hit the MINE and lost 1 point,                            null, JOptionPane.ERROR_MESSAGE);                    player.updatePoints(-1);                    return;                }            }                      // tell user how close he is to right answer                if (userGuess < randomNumber && userGuess != 0){                    JOptionPane.showMessageDialog(null, Too low!);               }else if(userGuess > randomNumber){                   JOptionPane.showMessageDialog(null, Too high!);               }else if(userGuess == randomNumber){                   JOptionPane.showMessageDialog(null, You win! You tried  + numGuesses +  time(s) total);                   player.updatePoints(numGuesses,option);                   options(player);               }               askForGuess();        }    }    public void askForGuess(){        // userGuess = Integer.parseInt(JOptionPane.showInputDialog(You are guessing random number between 1 and  + this.max + . Enter your guess: ));        userGuess = promptForInteger(You are guessing random number between 1 and  + this.max + . Enter your guess: );     }}Player.javaimport java.io.BufferedReader;import java.io.File;import java.io.FileNotFoundException;import java.io.FileReader;import java.io.FileWriter;import java.io.IOException;import java.io.PrintWriter;import javax.swing.JOptionPane;public class Player {    protected String name;    protected int points=0;    protected String txtPathPlayer = src/Player.txt;    protected String txtPathPoints = src/Points.txt;    public Player() throws IOException{ // constructor, I want to run this every time someone creates new player            getName(); // load username from Player.txt if it doesn't exists ask for username            loadPoints();    }    public void getName() throws IOException{        FileReader in;        boolean nameFound = false;       if (!fileExists(txtPathPoints)){          askName();       }        try {            in = new FileReader(txtPathPlayer);            BufferedReader br = new BufferedReader(in);            String line;            while ((line = br.readLine()) != null) { // search if there is saved username in Player.txt and store it in name                if (line.contains(username: )){                    name = line.substring(10);                    nameFound = true;                }            }            if (!nameFound){ //if name is not stored in text file ask for name                askName();            }            br.close();        } catch (FileNotFoundException e) {            e.printStackTrace();        }    }    public void askName() throws IOException{            try {                FileWriter writer = new FileWriter(txtPathPlayer, true);                name = JOptionPane.showInputDialog(Please enter your name: );                writer.write(username:  + name);                writer.close();            } catch (IOException e) {                e.printStackTrace();            }    }    public boolean fileExists(String Path){        File f = new File(Path);        if (!(f.exists() && !f.isDirectory())) {             return false;        }else {           return true;        }    }   public void loadPoints() throws IOException{       FileReader in;       boolean pointsEmpty = true;       if (!fileExists(txtPathPoints)){           updatePoints(0);       }       try { // look if there is points.txt file and load points to variable points            in = new FileReader(txtPathPoints);            BufferedReader br = new BufferedReader(in);            String line;            while ((line = br.readLine()) != null) {                if (line.contains(points: )){                    points = Integer.parseInt(line.substring(8));                    pointsEmpty = false;                }            }            br.close();        } catch (FileNotFoundException e) {            e.printStackTrace();        }       if (pointsEmpty){ // if point.txt does exist but has invalid data update points to 0           updatePoints(0);       }   }    public void updatePoints(int points) throws FileNotFoundException{        if (points < 0 && this.points < 0){            JOptionPane.showMessageDialog(null, You don't have any points so you lose anything! :) );        }else {            this.points += points;            PrintWriter pw = new PrintWriter(txtPathPoints);            pw.print(points:  + this.points);            pw.close();        }    }    public static boolean isBetween(int x, int lower, int upper) {          return lower <= x && x <= upper;        }    public void updatePoints(int numGuesses, int option) throws FileNotFoundException{        if (option == 1){              switch (numGuesses) {            case 1:                 updatePoints(7);                JOptionPane.showMessageDialog(null, You won 7 points!);                break;            case 2:                 updatePoints(5);                JOptionPane.showMessageDialog(null, You won 5 points!);                break;            case 3:                 updatePoints(3);                JOptionPane.showMessageDialog(null, You won 3 points!);                break;            default:                updatePoints(1);                JOptionPane.showMessageDialog(null, You won 1 point!);             }        }        if (option == 2){             if (isBetween(numGuesses, 1, 3)) {                updatePoints(7);                JOptionPane.showMessageDialog(null, You won 7 points!);            }else if (isBetween(numGuesses, 4, 6)) {                updatePoints(5);                JOptionPane.showMessageDialog(null, You won 5 points!);                       }else if (isBetween(numGuesses, 7, 9)) {                updatePoints(3);                JOptionPane.showMessageDialog(null, You won 3 points!);                       }else{                updatePoints(1);                JOptionPane.showMessageDialog(null, You won 1 point!);            }        }        if (option == 3){             if (isBetween(numGuesses, 1, 10)) {                updatePoints(7);                JOptionPane.showMessageDialog(null, You won 7 points!);            }else if (isBetween(numGuesses, 11, 20)) {                updatePoints(5);                JOptionPane.showMessageDialog(null, You won 5 points!);                       }else if (isBetween(numGuesses, 21, 30)) {                updatePoints(3);                JOptionPane.showMessageDialog(null, You won 3 points!);                       }else{                updatePoints(1);                JOptionPane.showMessageDialog(null, You won 1 point!);            }        }    }}
Guess a number game with mines version 3
java;beginner;game;swing;number guessing game
First I just want to say I don't have a lot of time at the moment to write a full review, but I'll point some things I noticed by taking a quick first pass over your code and then edit my post tomorrow when I have some more time to take an in-depth look. EDIT: I added more at the end of my original post, please see below.In your Game class, method playGame - this loop (below), while technically valid is not the standard pattern for a for loop which makes it a little harder to read for anyone else looking at your code. Readability is very important for long term maintenance and although this is a small program you said you wanted to learn how to write good code in general.for (int numGuesses = 0; true; ){I would replace this with something more readable by breaking it into two lines: int numGuesses = 0; while(true){Also in the Game class, in the options method you have:option = promptForInteger(Hi  + player.name + ! \n    + Please select your game:\n1) Guess number between 1 and 10 with 1 mine.\n    + 2) Guess number between 1 and 20 with 3 mines.  (You need 20 points to play this)\n    + 3) Guess number between 1 and 100 with 20 mines.  (You need 100 points to play this)\n    + 4) Exit the game (no points needed for this :) )    + \n \n \n you currently have:  + player.points +  points \n \n);This could potentially be executed many times. It would be more efficient to use StringBuilder in situations when you have a lot of string concatenation that is repeated many times. As long as I'm writing about this method, generally a method name should describe the purpose of the method - what action is it performing? I would suggest a better name, for the sake of readability, would be something like promptForGameOptions. Furthermore I see that this method calls these other two methods: createNumberAndMines();playGame(player);Each method should have a very clear purpose and generally that purpose should be to do one specific task. When you have options calling these methods it feels as though options does not have a clear purpose because it mixes view logic with controller logic. I suggest a more appropriate place for these method calls would be in Main.main because the main method's purpose is to act as a controller for the game. If you're not familiar with the MVC (model-view-controller) pattern, you can read about it on wikipedia. In your Player class you have a lot of file reading/writing logic that would probably be better placed in a separate utility class. I say this because as it is written now you're mixing controller logic with data model and that can lead to code that is harder to maintain in the long run. Try to keep your data separate from the logic that loads and uses the data.I also notice that in your file reading logic within the Player class you're not handling the scenario where the data in the file is invalid in some unexpected way. For example you have: if (line.contains(username: )){    name = line.substring(10);    nameFound = true;}but you never handle the situation where the line contains only username:  and nothing else. If the line is username:  then name =  after you run this code. Also, are you intending for there to be multiple usernames in this file? If not you could just break out of the loop or forgo the loop entirely. That's all I have for now, hope that gives you something to work with until I can take a closer look.EDIT:As promised, here are a few more items I see upon taking a closer look at the code you posted. I'll start with the Game class. In the options method there is a bug. You tell the user they need 20 points to play option 2 but the logic checks for 10 points.+ 2) Guess number between 1 and 20 with 3 mines.  (You need 20 points to play this)\n...case 2: if (player.points < 10){In the method createNumberAndMines I see you're using the following loop structure: for (int i = numberOfMines; i >= 0; i--)Like I mentioned above with the other for loop this is technically correct, but unless there is a good reason to do this the standard pattern for a for loop should be used to improve readability: for(int i = 0; i < numberOfMines; i++)This would also help make Game.options more readable since you could set the value of numberOfMines to 1, 3, and 20 instead of how you have it now: 0, 2, 19.Looking more at this createNumberAndMines method I have additional concerns. The name of the method would seem to say that its purpose is to initialize the location of the mines, yet it asks for the player's guess in the beginning. This is another example of mixing view with controller logic. Also, the following line is confusing and I think it's mostly due to the variable name: randomNumber = getRandom(); // get random number to guessIf you named this variable something like correctAnswer it would be much more clear to anyone reading the code what its purpose is. Finally, there is nothing wrong with using an array to store the mines but if you used an implementation of the List object you wouldn't need to write so much code for things like checking if a number has already been added to the mines. Here's an example of how you might rewrite some of the logic of this method to use an ArrayList: List<Integer> mines = new ArrayList<Integer>();...for(int i=0; i < numberOfMines; i++){     //Assume the mine will be invalid     boolean isValidMine = false;    //generate a mine and add it to the list of mines if it is valid.     do{          int mine = getRandom();         //a mine is valid if it is not already in the list, does not equal the correct answer, and is not the first guess made by the user         isValidMine = !mines.contains(mine) &&  mine != correctAnswer && mine != userGuess;         if(isValidMine)             mines.add(mine);     }while (!isValidMine); }I really only bring up the above point because it's important for Java programmers to become familiar with the Java Collections Framework. If you haven't studied it yet, here is a link to get you started. Note that having the ability to call the contains method also simplifies your code in your Game.playGame method as well because you can replace: for (int i=numberOfMines; i>=0; i--){// check if user hits mine    if (mine[i] == userGuess){        ...    }}   with:if(mines.contains(userGuess)){    ...}If you're not comfortable with collections you can write your own contains method and it will simplify your code in much the same way: public boolean minesContains(int element){    for(int i = 0; i < mine.length; i++){        if(mine[i] == element){            return true;        }    }    return false;}The reason I emphasize this so much is that one of the fundamental practices of Object Oriented Programming is to reuse code as much as possible by making it modular and generic. Since there are multiple places in your code that you use a containment check you should write a generic routine that you can call multiple times instead of rewriting the same logic over and over. This helps reduce errors, improve readability, and reduce maintenance. Moving on to the Player class, it appears you're trying to create some constants: protected String txtPathPlayer = src/Player.txt;protected String txtPathPoints = src/Points.txt;In general if you're creating a constant you use the keywords static and final and name it with all capital letters with underscores to separate words - for example: TXT_PATH_PLAYER. You can read this page for more details about why this is done.This is somewhat a personal preference but I find that positive expressions are more easy to read than negative ones. For example in your fileExists method you have this: if (!(f.exists() && !f.isDirectory())) {     return false;}else {   return true;}This forces me to stop for a moment and think through what the logic is doing. If it were written like this: if (f.exists() && !f.isDirectory()) {     return true;}else {   return false;}I can very easily see that it is going to return true when a file exists and is not a directory. You could even simplify this method more by directly returning the result of your boolean expression like this: return f.exists() && !f.isDirectory();Looking at the updatePoints method I think the different game modes or options as they're called in the code deserve to be represented as objects. The updatePoints logic is handling the calculation of points for each mode when, if you had an object to represent each mode, the objects themselves could have a method that takes numGuesses as a parameter and returns the number of points the player earned. Also, the Game.options method would be simplified as each object could have its own value for numberOfMines and max. If you really feel ambitious you could use an enum to represent them since there is a fixed number of them and they do not change.In conclusion I think you've done some great work so far. You have a good start on a modular design and most of the code is easy to read. Just don't be afraid to go back and change names of methods and variables. Try to think from an outsider perspective and consider the purpose of each variable or method when you name them. Remember that sometimes less is more - a simple method is easier to maintain than a complex one. If you have a complex task try to break it into smaller reusable pieces. To help with debugging I recommend you either throw in some System.out.println statements or research some java logging frameworks like SLF4J, Apache Commons Logging, log4j, logback, etc. Logging can be a huge help - especially in larger projects. Another thing you should consider is learning about JUnit and creating some automated tests for your code.Take care!