77
\$\begingroup\$

I always get marked down for my comments and I just wanted to see if these comments are acceptable or what I should include/where I should include them.

import java.util.*; //so I can use scanner

public class GuessingGame {
   public static void main(String[] args) {

      Random rand = new Random ();
      int max = 100; //sets limit on the random number generator
      Scanner input = new Scanner(System.in);
      int guess;//so I can compare console input to random number
      boolean play = true;
      int totalGames = 0;  
      int totalGuesses = 0; 
      int bestGame = Integer.MAX_VALUE; 

      System.out.println("Can you guess the word?");
      System.out.println("I am sure you cannot guess!");
      System.out.println("Go ahead and try!");
      System.out.println();

      while (play) { //so game will restart after a win

         System.out.println("I'm thinking of a number between 1 and " + max + "...");
         int numberToGuess = rand.nextInt(max) + 1;
         int numberOfTries = 0;
         //so user can continue guessing until correct
         boolean win = false;  
         while (!win) {

            System.out.print("Your guess? " + numberToGuess);
            guess = input.nextInt();
            numberOfTries++;

            if (guess == numberToGuess) {
               win = true;  
            } else if (guess > numberToGuess) {
               System.out.println("It's lower.");
            } else if (guess < numberToGuess) {
               System.out.println("It's higher.");
            }      
            input.nextLine();
         }

         if (numberOfTries == 1) {
            System.out.println("You got it right in " + numberOfTries + " guess!");
         } else {
            System.out.println("You got it right in " + numberOfTries + " guesses!");
         }   

            totalGames++;
            totalGuesses+= numberOfTries;
            System.out.print("Do you want to play again? ");

            //so user can choose to play another game    

            String answer = input.nextLine();
            char firstLetter = answer.charAt(0);
            if (firstLetter == 'y' || firstLetter == 'Y') {
            play = true;  
            } else {
            play = false;                               
         } 


         bestGame = Math.min(bestGame, numberOfTries);
         System.out.println();               
      }

      System.out.println("Overall results:");
      System.out.println("Total games   = " + totalGames);  
      System.out.println("Total guesses = " +  totalGuesses);
      System.out.printf("Guesses/game  = %.1f \n", totalGuesses/(double)totalGames);
      System.out.println("Best game     = " + bestGame);
   }  
}
\$\endgroup\$
  • 16
    \$\begingroup\$ If I ever become a programming teacher, please remind me to not care about code comments! \$\endgroup\$ – Simon Forsberg May 8 '15 at 23:58
158
+100
\$\begingroup\$

Welcome to the hell of comments, and personal taste. There are multiple answers to your core question that may be right:

  1. You have not commented enough.
  2. You have commented all the wrong things.
  3. You have commented too much.
  4. Your comments are in the wrong format.
  5. You are missing the formal comments.

To be clear though, nothing we can say will be in line with what your marks are based on, unless you can tell us why you were marked down... ;-)


About Comments in general

OK, comments are supposed to make plain what the code does not tell us already.

Let me say that again.

Comments are supposed to make plain what the code does not tell us already.

Now, whether you are marked on that, or not, is a different story. I have found that the academic application of marks for comments is often contrary to the need for comments... particularly when the code is good code.

Good code seldom needs comments.

Let me say that again:

Good code seldom needs comments.

So, follow this logic through, and then follow the inverse back...

  1. Comments make plain what the code does not tell us already.
  2. Good code seldom needs comments.

thus....

  1. Good code seldom needs comments.
  2. Good code must make plain what the code does without the need for additional explanation.

Applied to your code

Let's start at the top....

import java.util.*; //so I can use scanner

Now, if your code was:

import java.util.Scanner;

then it would not need the comment.

Your next comment is here:

  int max = 100; //sets limit on the random number generator

Well, that is, actually, not a horrible comment, or code. But, it could be better, if we gave it a decent name:

int randomGeneratorLimit = 100;

It would be even better if we made that magic number a constant:

private static final int RANDOM_GENERATOR_LIMIT = 100;

Now, no need for a comment at all.

Next up, this line:

  while (play) { //so game will restart after a win

Well, this one is interesting.... the comment is actually needed here because of the loop you are using.

If you convert this to a do-while loop, and use some functional extraction... then you can convert your code to a self-documenting, commentless block:

private static boolean playAgain() {
    System.out.print("Do you want to play again? ");
    String answer = input.nextLine();
    char firstLetter = answer.charAt(0);
    return firstLetter == 'y' || firstLetter == 'Y';
}

then you can use that function as follows:

do {

    ....... // game logic

} while (playAgain());

No need for a comment (or a play variable).

Similarly, you have the code:

     //so user can continue guessing until correct
     boolean win = false;  
     while (!win) {

This should be a do-while as well, and should look like:

do {
     ..... // game logic
} while (guess != numberToGuess);

Again, that makes the logic clear, no need for a comment.

I believe that has now eliminated all of your comments..... and replaced them with code that does not need a comment, because the code is self-explanatory.

In other news, if you had more functions, with good names, then your code would be simpler to read as well.

Bottom line, though, is that the comments in your code should fill in the blanks that your code does not. In addition, your comments should give details on the motivation, and not the application of your code. You should, in general, comment only on why your code does things, not what your code is doing.

That leads on to the other comments you are missing... JavaDoc.

JavaDoc is documentation that should explain what your code does at an abstract level.

You have no JavaDoc, and you likely should. JavaDoc is where you describe what your code does, because, typically, the people reading the javadoc are not reading the code, so they need something else to tell them what the code does.

\$\endgroup\$
  • 16
    \$\begingroup\$ An excellent answer regarding comments! This will be my favourite reference from now on to link in answers. \$\endgroup\$ – Heslacher May 8 '15 at 4:42
  • 7
    \$\begingroup\$ Excellent. Document the "why" not the "how," the code should make the "how" as obvious as possible. \$\endgroup\$ – Chad Schouggins May 14 '15 at 20:06
  • 8
    \$\begingroup\$ Fantastic answer. I love how for each comment you come up with a perfect example of how to get rid of it, using the OP's original example. Also very good points about JavaDoc. Ask yourself, how many libraries do you use via Javadoc and how many via actually reading the source... ever read the source code of Object.toString? And if your code does not have JavaDocs, how usable does that make it for others with a similar taste in not reading all the source code of all the libraries they use? \$\endgroup\$ – Stijn de Witt Dec 8 '15 at 23:14
  • 1
    \$\begingroup\$ Totally OT, but System.out.print("Do you want to play again? "); immediately made me think of WarGames. We now return you to your regularly scheduled Code Review... \$\endgroup\$ – FreeMan Mar 17 '17 at 17:05
  • 2
    \$\begingroup\$ Very nice. Most people recommend writing better comments but you recommend writing better code so comments become obsolete. \$\endgroup\$ – yuri Mar 1 '18 at 12:21
45
\$\begingroup\$

So you want to write good comments.

Good comments...

  • ...say why, not what.
  • ...don't state the obvious or rephrase what the code is already saying.
  • ...don't turn into lies if the code changes.

Let's see...

import java.util.*; //so I can use scanner

The problem is that you're importing an entire library, and specify that you're only doing that for one of its types. Either you're lying and you're actually using more than that, or the import statement is overkill.

int max = 100; //sets limit on the random number generator

Comments on variables usually indicate poor naming, because good identifiers are self-explanatory. If max was called maxRandomValue, would this comment be needed?

int guess;//so I can compare console input to random number

This is another comment that used in-place of a more meaningful identifier. If guess was called userInput, would this comment be needed?

while (play) { //so game will restart after a win

There's no reason to have this comment either; it's pretty clear what the intent is here. The only potential issue I'm seeing with while (play) is that play is a verb, when a Boolean identifier is clearer when it starts with is or has - in this case, while(isPlaying).

 //so user can continue guessing until correct
 boolean win = false;

This comment isn't needed either. Again it's pretty clear what the intent is, but then again, it looks like a verb and could be called isCorrectGuess.


This is an interesting one:

//so user can choose to play another game    

    String answer = input.nextLine();
    char firstLetter = answer.charAt(0);
    if (firstLetter == 'y' || firstLetter == 'Y') {
    play = true;  
    } else {
    play = false;
} 

The indentation is clearly off here, and it's confusing. Consider:

    String answer = input.nextLine();
    char firstLetter = answer.charAt(0);
    if (firstLetter == 'y' || firstLetter == 'Y') {
        play = true;  
    } else {
        play = false;
    } 

However you're using a conditional to assign true in one branch, and false in the other - the whole if block is therefore useless:

    String answer = input.nextLine();
    char firstLetter = answer.charAt(0);
    play = firstLetter == 'y' || firstLetter == 'Y';

Now, about the comment...

//so user can choose to play another game    

This one is formulated in a way that makes it look like it's saying why, but it basically says "this chunk of code does [...]". Whenever that happens, the rule of thumb is that you should extract that chunk into its own method.


Bottom line, I don't think any of these comments are really needed. And do yourself a favor, drop that "so abc can xyz" formulation.

\$\endgroup\$
  • 3
    \$\begingroup\$ I think playing is a better variable name than isPlaying. isPlaying is for boolean getter-methods. Being a verb is not a problem for a boolean variable name IMO. (I think the Java conventions agree with me on this one) \$\endgroup\$ – Simon Forsberg May 9 '15 at 0:02
26
\$\begingroup\$

Bug

System.out.print("Your guess? " + numberToGuess);

Wells I guess I'm going to guess it right the first time, every time... :(

Code Review

@rolfl and @Mat's Mug have provided valuable insights on your question about comments, so I'll attempt to tackle the code part of it...

Use of boolean flags

Currently, both your loop conditions depend on setting boolean flags, which does make the code easier to read. Alternatively, since there are only two specific conditions when you want to exit from both loops, you can also consider doing a break instead at:

  1. Guessing the right number

    if (guess == numberToGuess) {
        break;
    }
    
  2. Exiting from the game

    if ( <first character of input is y/Y> ) {
        break;
    }
    

Thereafter, your while-loops will just be while (true) { ... }.

edit @rolfl's approach is better and I'll recommend it over the above.

Validating user input

guess = input.nextInt();
...
input.nextLine();

nextInt() throws a InputMismatchException if a non-integer input is entered, and not only that you must remember to 'consume' the current line by doing a nextLine() at the end. I'll suggest reading the entire line in, do a Integer.parseInt(line) and catching the NumberFormatException for invalid inputs. You can then repeatedly prompt the user until you get a valid integer (maybe you'll want to exclude negative values, or numbers outside your specified guessing range).

Printing this or that

There are two places where you are doing something similar to the following:

if (condition) {
    System.out.println("..." + "x" + "...");
} else {
    System.out.println("..." + "y" + "...");
}

Why not put this into a method that can abstract away this logic?

private static void printEither(boolean condition, String format, 
                                    String ifTrue, String ifFalse) {
    System.out.printf(format + "%n", condition ? ifTrue : ifFalse);
}

The assumption is that format has only one placeholder "%s", and format need not contain a trailing newline as the method is adding it. Usage:

printEither(guess > numberToGuess, "It's %s.", "lower", "higher");
...
printEither(numberOfTries == 1, "You got it right in " + numberOfTries + " %s!", 
                                    "guess", "guesses");

Do you want to play again?

The comparison can be simplified as such:

if (input.nextLine().substring(0, 1).equalsIgnoreCase("y")) {
    break;
}

try-with-resources

You should also use try-with-resources if you're on Java 7 on your Scanner.

\$\endgroup\$
  • 1
    \$\begingroup\$ I don't like the line input.nextLine().substring(0, 1).equalsIgnoreCase("y"), too many chained calls. Moreover it throws exception if an empty line will be written. I'd extract the logic of "do you want to play again?" in a separate method. \$\endgroup\$ – user131519 May 29 '17 at 9:40
19
\$\begingroup\$

Curiously, your introductory text says "Can you guess the word?" I thought this was a , not .

Your comments are mostly innocuous. The worst one is //so I can use scanner, which is a bit misleading, because you also need the import for java.util.Random. You would have been better off writing

import java.util.Random;
import java.util.Scanner;

… with no comment there.

My main concern is the length of the main() function — 70 lines. The sheer length of the code makes it hard to see and understand all of the code at once. By breaking it up into functions, you would allow each function to be analyzed separately, and you can also give a name to each chunk of functionality. If you pick good names, then the program can be somewhat self-documenting. If you write JavaDoc, then that's an even more effective type of comment.

In addition, you should avoid relying on flag variables like play and win to control program flow. Write your loops properly, and if necessary, use keywords like continue or break to get to where you want to go.

Note that if you just press Enter at the "Do you want to play again?" question, your program will crash with a StringIndexOutOfBoundsException.

import java.util.Random;
import java.util.Scanner;

public class GuessingGame {
    private static final int MAX = 100;
    private final Random rand = new Random();
    private final Scanner input;

    public GuessingGame(Scanner input) {
        this.input = input;
    }

    /**
     * Picks a number and asks for guesses until a correct guess.
     *
     * @return The number of guesses used (1 if guessed on the first try)
     */
    public int play() {
        System.out.println("I'm thinking of a number between 1 and " + MAX + "...");
        int numberToGuess = rand.nextInt(MAX) + 1;
        int score;
        for (score = 1; ; score++) {
            System.out.print("Your guess? ");
            int guess = input.nextInt();

            if (guess == numberToGuess) {
               break;
            } else if (guess > numberToGuess) {
               System.out.println("It's lower.");
            } else if (guess < numberToGuess) {
               System.out.println("It's higher.");
            }      
            input.nextLine();
        }
        System.out.printf("You got it right in %d guess%s!\n",
                          score,
                          score == 1 ? "" : "es");
        return score;
    }

    /**
     * Asks the user if he/she wants to play again.
     *
     * @return true for a "yes" answer, false for a "no" or unrecognized answer
     */
    public boolean playAgain() {
        System.out.print("Do you want to play again? ");
        String answer;
        while ((answer = input.nextLine()).isEmpty());
        char firstLetter = answer.charAt(0);
        return (firstLetter == 'y' || firstLetter == 'Y');
    }

    /**
     * Plays number-guessing games until user decides to quit,
     * then displays statistics.
     */
    public static void main(String[] args) {
        try (Scanner input = new Scanner(System.in)) {
            System.out.println("Can you guess the number?\n" +
                               "I am sure you cannot guess!\n" +
                               "Go ahead and try!\n");

            int guesses = 0, gamesPlayed = 0, bestScore = Integer.MAX_VALUE;
            GuessingGame game = new GuessingGame(input);
            do {
                int score = game.play();
                bestScore = Math.min(bestScore, score);
                guesses += score;
                gamesPlayed++;
            } while (game.playAgain());

            System.out.println("Overall results:");
            System.out.printf("Total games   = %d\n", gamesPlayed);  
            System.out.printf("Total guesses = %d\n", guesses);
            System.out.printf("Guesses/game  = %.1f\n", guesses/(double)gamesPlayed);
            System.out.printf("Best game     = %d\n", bestScore);
        }
    }
}
\$\endgroup\$
18
\$\begingroup\$

Your code is missing several very important comments. Some of the comments you do have are lies, others are trivial, yet others don't say what you should be saying.

The most important comment of all

(From the perspective of the lawyers)

// Copyright 2015 Eric Irwin. All rights reserved.

This doesn't matter so much in the world of classroom assignments. In the working world, it can matter a lot. Some people elaborate on what rights are reserved. Why? Reserve all rights. Or maybe refer to the license agreement. Keeping it short and simple makes it easier to insert this very important comment.

The most important comments of all

(From the perspective of javadoc)

Every class, every function should have a javadoc comment. You do not (you should not!) go overboard. Do not say when you wrote it, or even who wrote it. Write, in plain English (or plain French, or German, or whatever) why the class or function exists.

In the case of a class, say why the class exists, how to use it. Most classes have multiple member functions, multiple data members. An external user who calls those member functions in a random order is most likely to be toast. How member functions need to be sequenced, how they interact with one another: That's the kind of stuff you want to put into that class javadoc comment.

In the case of a function, document what the function does, why it exists -- in plain English. Document the arguments to the function and the return value from the function. In one case (main), that might be blatantly obvious, but a program only has one main. The arguments to main oftentimes are tricky as can be. In this case, you aren't using the arguments -- so say so!

Other missing comments

Your comments show small things but miss the big picture. A perfect example:

while (play) { //so game will restart after a win

Much better, in my opinion:

// Play game after game until the user decides to stop.
while (play) {

That outer loop exists so a user can play multiple games. (This was probably a part of the assignment. So say that.) That's the big picture. So game will restart after a win? That's not "big picture". While well-written code can explain itself to some extent, it does not explain the big picture, aka intent or architecture.

End-of-line comments versus full-line comments

Most of your comments are end-of-line comments. This style of commenting forces people to be terse; more often than not, overly terse. It also encourages people to comment the obvious (e.g., i++; // increment i). The only thing worse than i++; // increment i is a comment that is inconsistent with the code.

Commenting the obvious with an end-of-line comment is easy, but your code for the most part should be obvious. A comment on the obvious is not needed. Commenting the not so obvious? End-of-line comments typically don't do that very well.

\$\endgroup\$
  • 5
    \$\begingroup\$ My perspective on commentary is atypical. I write software that, done incorrectly, might kill people or risk hundreds of millions of dollars. Comments are important. They need to say why things exist, why a certain path was chosen, what things do (but at a big picture level). They also cannot be whimsical. There is no room for whimsy in software that can kill, thanks to those wonderful critters that are members of the subhuman species otherwise known as "class action lawyers." \$\endgroup\$ – David Hammen May 10 '15 at 4:20
  • 5
    \$\begingroup\$ For the record, writing All rights reserved. in a comment and then posting to Code Review would be counter-productive, as when you post here it is automatically posted under the CC-BY-SA license. \$\endgroup\$ – Simon Forsberg May 12 '15 at 22:19
  • \$\begingroup\$ "As you can see jury, the defendant wrote '// might explode' in his source codes. What exactly do you think was going through his head at that time." \$\endgroup\$ – LegendLength Jul 15 '17 at 20:08

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.