5
\$\begingroup\$

I'm a beginner in the coding world, and I've been learning Java recently. I tried to make a small text game with the limited knowledge that I currently have of Java. I would like you to tell me ways to improve my code, so that I can learn it better. I currently feel my code is a bit messed up, but at least it's a start.

import java.util.*; public class GameMain {
public static void main(String []args) {

    Scanner rep = new Scanner(System.in);  //Scanner variable
    String nm;                             //Stores the name of the player.
    String d1,d2,d3;                       //Store the decisions of player.
    int age;                               //Stores the age of player.
    String line2, line3, line4;            //Text for the lines.

    //The next few lines contain text for the dialogues and lines.
    line2 = "What in the.......!!! *He shoots you in the head.* GAMEOVER";
    line3 = "Good. Lets GO! *As you both proceed, suddenly a masked man enters the \ncell with a gun and holds the man at gunpoint. You start to shit your pants*";
    line4 = "You slowly raise your arms in surrender. As you are getting down on your knees, another man shouts from\n15"
            + " outside'HEY! VIKTOR! MAKE IT QUICK!'. The man gets distracted. You see this as an opportunity";

    //Next few lines enforce Age Limit.

    System.out.println("Please enter your age.");
    age = rep.nextInt();

    if (age>13) {

        //This stores the name of the player to the variable nm.
        System.out.println("Hello. Please Enter your Name :");
        }else {
            System.out.println("You are not old enough.");
            System.exit(0);
        }

    nm=rep.next(); /*allows the user to enter a name, if he/she is older than 13*/


    //Next lines print out the first dialogues and decision.
    System.out.print("[note-type exit at any time to exit] Wake UP!      Goddamnit!\n Looks like they overdosed you a bit. You are ");
    System.out.print(nm);
    System.out.println(" right? Cmon, lets get the hell out of here.");
    System.out.println("What do you do? A. Punch the guy in the nuts.");
    System.out.println("B. Follow him.");
    d1 = rep.next();

    switch(d1) {

    case "exit":
        System.exit(0); /*I decided to let the user terminate the program if            he wants by this command.*/
        break;

    case "A" :
        System.out.println(line2);
        System.exit(0);
        break;
    case"B":
        System.out.println(line3);
        break;

    }

    System.out.println("What do you do? ");
    System.out.println("A. You try to take the gun from the man.");
    System.out.println("B. You slowly raise your arms to display that you pose no danger.");

    d2=rep.next();

    switch(d2) {

    case "exit":
        System.exit(0);
        break;
    case "A":
        System.out.println(" He shoots you in the head when you attempt to take his gun. *GAME OVER*");
        System.exit(0);
        break;
    case "B":
        System.out.println(line4);
        break;
    }

    System.out.println(" What do you do?\n A.Punch the man in the nuts and take his gun.\n B.Make a run for it through the door.\n C.Do nothing and remain in the surrender position.");

    d3=rep.next();

    switch(d3) {

    case "exit":
        System.exit(0);
        break;

    case "A" :
        System.out.println(" You succesfully punch him in the nuts. He shouts in agony and you easily grab the gun that slipped form his hands.\n You now have the gun, What do you do?");
        break;

    case "B" :
        System.out.println(" You try to make a run for it through the door, but the man notices your escape attempt.\n He immediately shoots at you, but misses. He shoots at you again. It hits you right in the bottoms and you fall on the ground,\n shouting in agony. The man then shoots your buddy in the head and proceeds to do the same to you.*GAME OVER*");
        System.exit(0);
        break;

    case "C" :
        System.out.println("You decide its better to comply. You and your buddy are down on your knees. The man then turns to you buddy\nand says\"Hope you had a fun little life. HA! just kidding! Hope you DIDNT\"\n*BOOM* Your friends severed head lies next to you. The man now shifts his gaze onto you.\n What do you do? ");

//The game is not complete but I only made this to learn. I would love if you guys told me my mistakes and things i could've done differently and better. I would appreciate it . } 

}

}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ You can break up the code into different functions to make it more readable and maintainable. For example where your checking for age,can become a function by itself \$\endgroup\$ – DacorieS Sep 17 '15 at 4:48
11
\$\begingroup\$

Welcome to Codereview. It's great that you're doing something to learn and trying to make it fun, keep it up!

Use meaningful variable names.

d1,d2,d3 cost little to simply name decision1 or better yet punchDecision or followDecision same for nm to just name or playerName, it'll help you down the line if this becomes a big project and is a generally good thing to pick up as early as possible.

Use line breaks for readability

It looks like you're familiar with the new line character, in a lot of your code you're calling print multiple times which has a cost, and in some other places you have some seriously horizontally unkempt lines, it helps if you split that up into several lines for readability.

For example your first line could be:

System.out.println(
   "What do you do? "
   + "\nA. You try to take the gun from the man."
   + "\nB. You slowly raise your arms to display that you pose no danger."
);

Follow Java Conventions

  • Write variables on their own line.
  • import statements should be at the top and separate from class declaration.
  • Everything within curly brackets should be tabbed, including the main method.
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Thank you! Those are really helpful suggestions! I really appreciate it. \$\endgroup\$ – Aryan Srivastava Sep 17 '15 at 3:34
  • \$\begingroup\$ String concatenation costs time too, right? \$\endgroup\$ – martijnn2008 Sep 17 '15 at 19:52
  • \$\begingroup\$ Far less than it will save even a single reader for the lifetime of the program and definitely less than a method call. \$\endgroup\$ – Legato Sep 17 '15 at 20:37
6
\$\begingroup\$

You have a lot of code there which repeats the same basic things.

  1. The player is given the description of a situation.
  2. The player is offered choices.
  3. The player makes a choice, which puts him in a new situation (taking him back to step 1).

In Java, the way to represent things which are essentially the same, only with different details, is though classes.

Imagind a Choice class which contains a text field (a String) and a result field (a Situation object - an instance of the Situation class).

Imagine a Situation class, which contains a description field (a collection of strings),a way of printing the description (calling println on each collection element in turn) and a collection of Choices. The Situation has a turn method which

  1. calls the description print method
  2. prints a list of the text of each Choice, labelling them A, B and so on, in sequence
  3. asks for input.
  4. returns the Situation contained in the result field of the chosen decision.

Once you have these classes and created each object, your game could consist simply of a loop which takes the first situation, runs its action method and then runs the action of the returned situation and so on.

Note that I haven't told you which kind of collections to use for any of those fields. Lists, arrays, sets or maps could all be used. Which do you think would work best for Situation.description or Situation.decisions?

Another question to ponder is "What if different decisions (in the same situation or in different situations) have the same result, leading to the same situation? Would that need changes to the Situation or Decision class?" In my simple suggestion above, Situations describe both the action of the decision you make and the situation you are in after the action.

If you think about this carefully, you will realise that what I haven't explained is how you would end the program. There are several interesting ways to do that, but rather than just give them to you, I think you should refactor your game to make it use classes and then try and solve the problem yourself (maybe asking for help on a StackExchange site). Once you have working code, come back to this site and ask for a review.

EDIT OK, I'll give you a hint for the simplest solution - a Choice which has null in the result field. But I think that is the worst solution. There are much better ones.

\$\endgroup\$
4
  • \$\begingroup\$ I am a beginner in Java and some of what you said flew over my head like a plane. But, I tried to learn what lists and sets were, and I think (im like 80% sure im wrong), that its better to use lists?? . As for your second and very interesting question, im again lost (I started learning 4 days ago, dont judge me :P). Maybe, can we use an if statement to end the game?? \$\endgroup\$ – Aryan Srivastava Sep 17 '15 at 13:35
  • \$\begingroup\$ Yes, you could use an if statement to see if a turn returns null and stop the game if that is the case. \$\endgroup\$ – itsbruce Sep 17 '15 at 15:26
  • 1
    \$\begingroup\$ @AryanSrivastava Everybody was new to this once. Nobody is judging you :-) If lists, sets and maps are new to you, I think you could first try simplifying your code by using them, without creating any classes of your own. Then, you could look at how to simplify it even further with classes. \$\endgroup\$ – itsbruce Sep 17 '15 at 16:03
  • \$\begingroup\$ @AryanSrivastava If making classes seems difficult to you as well, try just making a few methods to break up your Main method. Build on that. After all most classes are just a collection of common methods :) \$\endgroup\$ – Robert Snyder Sep 17 '15 at 17:30
6
\$\begingroup\$

I'm suprised no one has mentioned that having all of your game in the Main method is also not good practice. So I will. Don't put all of your code in a single method, even more so don't put all of your code in the MAIN method. itsbruce mentions a great solution to this problem. So in my answer to you I want to show you the merits of Unit Tests. There is tons of info out there, and I'll never write it out as good as some of the posts/blogs/videos can. However I can show you that with this style game you'll see how beneficial it is quickly.

Personally I imagine that your game could be written in stages, and making different stage classes that do the 4 steps that itsbruce mentions. I also imaging a collection of those classes and looping through them all. If you make it to the end you win. a mock up of what I mean would be this

import java.util.Scanner;

public class GameMain {

    public void playGame() {
        for (GameStage stage : gameStages) {
            System.out.print(stage.getDescription());
            for (String decision : stage.getDecisions()) {
                System.out.print("    " + decision.toString());
            }
            String userInput = rep.next();
            stage.setUserInput(userInput);
            if (stage.isGameOver()) {
                System.out.print(stage.getGameOverText());
                return;
            }
        }
        System.out.println("Y0U R T3H W1NN3R!!");
    }

    static Scanner rep = new Scanner(System.in);  //Scanner variable

    public static void main(String[] args) {
        new GameMain().playGame();
    }
}

Now for what I consider to be the cool part. Unit testing. if each GameStage can be instantiated and run independant then you could easily go to any point in the game (without having to run through it) and test that your game logic is working properly. Lets just take your age verification part in and try it.

public class AgeVerificationGameStageTest {

    @Test
    public void whenUserToYoungGameIsOver() throws Exception {
        AgeVerificationGameStage stage = new AgeVerificationGameStage();

        stage.setUserInput("12");

        assertTrue(stage.isGameOver());

        stage.setUserInput("13");

        assertFalse(stage.isGameOver());
    }
}

well this is nice. This bit of logic runs in 29ms and tells me that I did not make a mistake with my logic. I'll show you how I did it.

public class AgeVerificationGameStage extends GameStage {
    @Override
    public String getDescription() {
        return "Please enter your age.";
    }

    @Override
    public void setUserInput(String userInput) {
        int age = Integer.parseInt(userInput);
        gameOver = age < 13;
    }

    @Override
    public String getGameOverText() {
        return "You are not old enough.";
    }

    @Override
    public Iterable<String> getDecisions() {
        return Collections.EMPTY_LIST;
    }
}

(sorry to you java people if getDecisions() doesn't stick with convention. I've been in the .NET world for too long... I would have returned new string[0] if it let me) This class is simple, direct, easy to digest and understand. When I go to start the game I know that all 13 and up year olds will be playing my game only. And if someone told me "I put 13 as my age and it said I couldn't play", or "I put 9 as my age and it let me play" I would know very quickly if that was a lie on their part, or if I introduced a bug... just run my test. If you want to put their claim to the test, just add it to the test or create a new test

@Test
public void userSaidTheyPut9AndTheGameWorked() throw Exception{
    AgeVerificationGameStage stage = new AgeVerificationGameStage();

    stage.setUserInput("9");

    assertTrue(stage.isGameOver());
}

There we go. Now if the user was telling the truth this test would fail. If it passed, well then there is a bug in our code. Good luck, and welcome to Code Review!

\$\endgroup\$
3
  • 3
    \$\begingroup\$ Seems the OP has some learning to do before he can appreciate this answer. He should definitely come back and reread it as he learns, though ;) \$\endgroup\$ – itsbruce Sep 17 '15 at 16:01
  • \$\begingroup\$ @itsbruce I agree! I feel that this is also the most important time to learn about Unit Testing since I believe it should be taught from day 1 either academically, or otherwise. \$\endgroup\$ – Robert Snyder Sep 17 '15 at 17:28
  • \$\begingroup\$ Haha, yes :). Its a bit too much currently but ill surely come back as i learn more. \$\endgroup\$ – Aryan Srivastava Sep 18 '15 at 10:26

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.