For the holidays I decided to go through my already learned stuff

[Jens Jensen]

Hi developers,

as for the holidays I decided to give myself a little break from gaining JAVA knowledge I decided to create a little program to strengthen my knowledge that I already have.
If you read the code below, do not think that I want to be a game designer of some sort, but from my point of view this was a very familiar topic to get my head around, so I decided to create a little "RPG fighting engine".

package dnd;

import java.util.Scanner;
import java.math.*;
import java.util.Random;

public class Main {
	static int playerHp = 100;
	static int shield = 20;
	static int enemyHp = 100;

	public static void main(String[] args) {
		// TODO Auto-generated method stub
		Scanner scanner = new Scanner(System.in);

		while (enemyHp > 0 && playerHp > 0) {

			// Options for the player to be printed on screen
			System.out.println("You are being attacked by a cactus! Beware!");
			System.out.println();
			System.out.println();
			System.out.println("Your health: " + playerHp);
			System.out.println("Enemies health: " + enemyHp);
			System.out.println();
			System.out.println("Please choose your action! Type the number!");
			System.out.println("1. Attack");
			System.out.println("2. Defend");
			System.out.println("3. Quit Game");

			int option = scanner.nextInt();
			if (option == 1) {
				attack();
				continue;

			} else if (option == 2) {
				defend();
				continue;

			} else if (option == 3) {
				quit();
				break;

			}

		}
		if (enemyHp <= 0) {
			System.out.println("You have defeated the enemy");
		} else if (playerHp <= 0) {
			System.out.println("You have been defeated");
		}

	}

	public static void attack() {
		int sword = new Random().nextInt(20);
		int enemySword = new Random().nextInt(15);
		if (sword == 0) {
			System.out.println("You missed!");
		} else {

			System.out.println("You attacked");
			System.out.println("You did " + sword + " damage.");
			enemyHp = enemyHp - sword;
		}

		if (enemySword == 0) {
			System.out.println("The enemey missed!");
		} else {
			System.out.println("The enemy attacked");
			System.out.println("The enemy did " + enemySword + " damage");
			playerHp = playerHp - enemySword;
		}

	}

	// TODO implement further methods
	public static void defend() {
		System.out.println("You defended");
		int enemySword = new Random().nextInt(15);
		if (enemySword == 0) {
			System.out.println("The enemy missed");
		} else if ((enemySword - shield) < 0) {
			System.out.println("You staggered the enemy attack and launched a counter attack");
			int sword = new Random().nextInt(20);
			System.out.println("You did " + sword + " damage.");
			enemyHp = enemyHp - sword;
		}

		else {
			System.out.println("The enemy attacked with " + enemySword + " damage");
			System.out.println("Your shield blocked " + shield + " damage");
			playerHp = playerHp - (enemySword - shield);
		}

	}

	public static void quit() {
		System.out.println("Thanks for playing!");
	}

}

This one is by no means complete, but I am happy that it at least works somehow


Best regards,
Jens

[Campbell Ritchie]

Sorry to appear negative, but I am afraid I can't see any object orientation in the design. Where are you creating a new Engine object (or Fighter object)? Why have you got so much code in the main() method?
Don't write println(...) repeatedly if you can combine several instructions into one printf(...) call. Remember you can replace println() with no argments by System.out.printf("...%n...").
Why are you using so many continues and breaks?

[Jens Jensen]

Hey Ritchie,

Only criticism makes us better. So thanks for sharing it.
For the object orientation I can say that I was simply sloppy. For the other parts I just do not know better. I have been working on java for the past 3 weeks only next to my job. So you can say that I am a bloody noob with no hands on experience. But I will improve the code. Thanks again for sharing

Best regards,
Jens

[Stephan van Hulst]

Split up your code in classes that are responsible for one thing and one thing only.

Don't mix logic and presentation. Console inout/output should be performed by a dedicated class that responds to events from your game engine.

Make your classes final. Designing your classes so they can be properly extended is REALLY difficult, so just make them final instead.

NEVER use static fields for anything other than holding constant values. The hp and shield fields should be instance fields encapsulated by a Player/Combatant/Creature class.

Make your fields private.

Don't use long if-else ladders. Either use a switch statement or use polymorphism.

Don't use break or continue statements redundantly.

Use proper variable names. A variable named 'sword' implies to me that it holds an instance of a Weapon class, not an integer. Name it 'damage' instead.

Use compound assignment operators.

[Carey Brown]

My nit...

enemyHp = enemyHp - sword;

should be

enemyHp -= sword;

[Monica Shiralkar]

Instead of having this big class, I think one can move all the static methods barring the main method to a Helper class and call them from the class.

[Stephan van Hulst]

Monica Shiralkar wrote:I think one can move all the static methods barring the main method to a Helper class and call them from the class.

Besides the main method, there shouldn't be any static methods at all!

[Campbell Ritchie]

Stephan van Hulst wrote:. . . Besides the main method, there shouldn't be any static methods at all!

And I would put the main method in a class all of its own, called GameDemo or similar.

[Monica Shiralkar]

Campbell Ritchie wrote:And I would put the main method in a class all of its own, called GameDemo or similar.

Means having a class GameDemo with main method and from the main method calling the static methods of this class?  If so, then it sounds similar to what I was thinking of when I talked about Helper Class.

[Campbell Ritchie]

Monica Shiralkar wrote:. . . a class GameDemo with main method and from the main method calling the static methods of this class?  . . .

No. See our FAQ/article.