Simulating character selection with JOptionPane

Question

I am writing a program to simulate some parts of the popular game Hearthstone by Blizzard Entertainment in Java. The first thing that happens in any game is the selection of the character each player will play as, called the "hero." I decided to use JOptionPane to create dialog boxes for this selection for the two players with the below code (which can also be found on GitHub here.

//A recreation of the game Hearthstone in Java
//Created 04/17/16 by Trevor B.
//Last updated 04/26/16
import java.util.*;
import javax.swing.*;
public class Hearthstone {
 public static void main(String[] args) {
 String redName, blueName;
 String redClass = null, blueClass = null;
 String redHero = null, blueHero = null;
 boolean redTurn, blueTurn; //for later
 /////////////////////////
 Scanner keyboard = new Scanner(System.in);
 Random randomizer = new Random(); //for later
 //Welcome message
 System.out.println("Hearthstone (Redux)\nBy Trevor B.\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
 //Assigning players
 System.out.print("Red player, enter your name: ");
 redName = keyboard.nextLine();
 System.out.print("Blue player, enter your name: ");
 blueName = keyboard.nextLine();
 //Picking hero
 while(redClass == null || redClass == "Choose one") {
 String[] heroes = {
 "Choose one", 
 "Druid", 
 "Hunter", 
 "Mage", 
 "Paladin", 
 "Priest", 
 "Rogue", 
 "Shaman", 
 "Warlock", 
 "Warrior"
 };
 redClass = (String) JOptionPane.showInputDialog(null, redName + ", pick your class", "Red class", JOptionPane.QUESTION_MESSAGE, null, heroes, heroes[0]);
 if(redClass == null || redClass == "Choose one") {
 System.err.println(redName + " did not pick a class!");
 }
 else {
 if(redClass == "Druid") {
 redHero = "Malfurion Stormrage";
 }
 else if(redClass == "Hunter") {
 String alternateHeroResponse = null;
 while(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 String[] alternateHeroResponseChoices = {
 "Choose one", 
 "Rexxar", 
 "Alleria Windrunner"
 };
 alternateHeroResponse = (String) JOptionPane.showInputDialog(null, redName + ", which hero do you want to use?", "Red hero", JOptionPane.QUESTION_MESSAGE, null, alternateHeroResponseChoices, alternateHeroResponseChoices[0]);
 if(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 System.err.println(redName + " did not pick a hero!");
 }
 else {
 redHero = alternateHeroResponse;
 }
 }
 }
 else if(redClass == "Mage") {
 String alternateHeroResponse = null;
 while(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 String[] alternateHeroResponseChoices = {
 "Choose one", 
 "Jaina Proudmoor", 
 "Medivh",
 "Khadgar"
 };
 alternateHeroResponse = (String) JOptionPane.showInputDialog(null, redName + ", which hero do you want to use?", "Red hero", JOptionPane.QUESTION_MESSAGE, null, alternateHeroResponseChoices, alternateHeroResponseChoices[0]);
 if(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 System.err.println(redName + " did not pick a hero!");
 }
 else {
 redHero = alternateHeroResponse;
 }
 }
 }
 else if(redClass == "Paladin") {
 String alternateHeroResponse = null;
 while(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 String[] alternateHeroResponseChoices = {
 "Choose one", 
 "Uther Lightbringer", 
 "Lady Liadrin"
 };
 alternateHeroResponse = (String) JOptionPane.showInputDialog(null, redName + ", which hero do you want to use?", "Red hero", JOptionPane.QUESTION_MESSAGE, null, alternateHeroResponseChoices, alternateHeroResponseChoices[0]);
 if(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 System.err.println(redName + " did not pick a hero!");
 }
 else {
 redHero = alternateHeroResponse;
 }
 }
 }
 else if(redClass == "Priest") {
 redHero = "Anduin Wrynn";
 }
 else if(redClass == "Rogue") {
 redHero = "Valeera Sanguinar";
 }
 else if(redClass == "Shaman") {
 redHero = "Thrall";
 }
 else if(redClass == "Warlock") {
 redHero = "Gul'dan";
 }
 else if(redClass == "Warrior") {
 String alternateHeroResponse = null;
 while(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 String[] alternateHeroResponseChoices = {
 "Choose one", 
 "Garrosh Hellscream", 
 "Magni Bronzebeard"
 };
 alternateHeroResponse = (String) JOptionPane.showInputDialog(null, redName + ", which hero do you want to use?", "Red hero", JOptionPane.QUESTION_MESSAGE, null, alternateHeroResponseChoices, alternateHeroResponseChoices[0]);
 if(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 System.err.println(redName + " did not pick a hero!");
 }
 else {
 redHero = alternateHeroResponse;
 }
 }
 }
 System.out.println(redName + " has chosen " + redClass + " and will play as " + redHero + ".");
 }
 }
 while(blueClass == null || blueClass == "Choose one") {
 String[] heroes = {
 "Choose one", 
 "Druid", 
 "Hunter", 
 "Mage", 
 "Paladin", 
 "Priest", 
 "Rogue", 
 "Shaman", 
 "Warlock", 
 "Warrior"
 };
 blueClass = (String) JOptionPane.showInputDialog(null, blueName + ", pick your class", "Blue class", JOptionPane.QUESTION_MESSAGE, null, heroes, heroes[0]);
 if(blueClass == null || blueClass == "Choose one") {
 System.err.println(blueName + " did not pick a class!");
 }
 else {
 if(blueClass == "Druid") {
 blueHero = "Malfurion Stormrage";
 }
 else if(blueClass == "Hunter") {
 String alternateHeroResponse = null;
 while(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 String[] alternateHeroResponseChoices = {
 "Choose one", 
 "Rexxar", 
 "Alleria Windrunner"
 };
 alternateHeroResponse = (String) JOptionPane.showInputDialog(null, blueName + ", which hero do you want to use?", "Blue hero", JOptionPane.QUESTION_MESSAGE, null, alternateHeroResponseChoices, alternateHeroResponseChoices[0]);
 if(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 System.err.println(blueName + " did not pick a hero!");
 }
 else {
 blueHero = alternateHeroResponse;
 }
 }
 }
 else if(blueClass == "Mage") {
 String alternateHeroResponse = null;
 while(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 String[] alternateHeroResponseChoices = {
 "Choose one", 
 "Jaina Proudmoor", 
 "Medivh",
 "Khadgar"
 };
 alternateHeroResponse = (String) JOptionPane.showInputDialog(null, blueName + ", which hero do you want to use?", "Blue hero", JOptionPane.QUESTION_MESSAGE, null, alternateHeroResponseChoices, alternateHeroResponseChoices[0]);
 if(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 System.err.println(blueName + " did not pick a hero!");
 }
 else {
 blueHero = alternateHeroResponse;
 }
 }
 }
 else if(blueClass == "Paladin") {
 String alternateHeroResponse = null;
 while(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 String[] alternateHeroResponseChoices = {
 "Choose one", 
 "Uther Lightbringer", 
 "Lady Liadrin"
 };
 alternateHeroResponse = (String) JOptionPane.showInputDialog(null, blueName + ", which hero do you want to use?", "Blue hero", JOptionPane.QUESTION_MESSAGE, null, alternateHeroResponseChoices, alternateHeroResponseChoices[0]);
 if(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 System.err.println(blueName + " did not pick a hero!");
 }
 else {
 blueHero = alternateHeroResponse;
 }
 }
 }
 else if(blueClass == "Priest") {
 blueHero = "Anduin Wrynn";
 }
 else if(blueClass == "Rogue") {
 blueHero = "Valeera Sanguinar";
 }
 else if(blueClass == "Shaman") {
 blueHero = "Thrall";
 }
 else if(blueClass == "Warlock") {
 blueHero = "Gul'dan";
 }
 else if(blueClass == "Warrior") {
 String alternateHeroResponse = null;
 while(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 String[] alternateHeroResponseChoices = {
 "Choose one", 
 "Garrosh Hellscream", 
 "Magni Bronzebeard"
 };
 alternateHeroResponse = (String) JOptionPane.showInputDialog(null, blueName + ", which hero do you want to use?", "Blue hero", JOptionPane.QUESTION_MESSAGE, null, alternateHeroResponseChoices, alternateHeroResponseChoices[0]);
 if(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 System.err.println(blueName + " did not pick a hero!");
 }
 else {
 blueHero = alternateHeroResponse;
 }
 }
 }
 System.out.println(blueName + " has chosen " + blueClass + " and will play as " + blueHero + ".");
 }
 }
 }
}

My main concern with this code is that a lot of code seems to be repeated, particularly assigning heroes where picking an alternate hero is an option (the process for selecting an alternate hero is the same in all cases; only the names are different).

Additionally, the code for the red and blue hero selections is the same, with the exception of the color in the method names. Is there a better way that this class can follow DRY?

• Arcturus: @Raystafarian The red player picks first. The heroes that are picked need not be distinct.

Answer 1

Assignement

I don't like multiple assignment per line. I find it difficult to discern what is value, what is a variable and add a little bit of mental compute time to read the code. Not a big deal, but I suggest you to keep one variable assignment per line.

String equality

The way you are doing String equality works but it's not the right way to do it. When you do redClass == "Paladin" what you are really comparing is address of object in memory. What is the trap in Java is that String are most of the time a constant and thus share the same address but let's try to break String equality :

String redClass = new String("Paladin");
System.out.println("Paladin" == redClass); //Output false
System.out.println("Paladin".equals(redClass));//Output true

So the lesson is to always use the method equals if you want to check for equality and use == if you want to make sure you have the same object.

Enum

If you don't know what an Enum is you should really learn it! Here is the Java tutorial for enum : https://docs.oracle.com/javase/tutorial/java/javaOO/enum.html.

So what can be an enum here ? Well the Hero class is a really good one! Let's see how I would do it :

public enum Classes {
 Druid("Malfurion Stormrage"),
 Hunter("Rexxar", "Alleria Windrunner"),
 Mage("Jaina Proudmoor", "Medivh","Khadgar"),
 Paladin("Uther Lightbringer", "Lady Liadrin"), 
 Priest("Anduin Wrynn"), 
 Rogue("Valeera Sanguinar"), 
 Shaman("Thrall"), 
 Warlock("Gul'dan"), 
 Warrior("Garrosh Hellscream", "Magni Bronzebeard");
 private String[] heroNames;
 Classes(String... heroNames) {
 this.heroNames = heroNames;
 }
 
 public String [] getHeroNames() {
 return this.heroNames;
 }
}

What is heroNames ? It's a small hack in my code that I'll present to you later to keep the abstraction at one level. Ideally, you should have a sub-enum or class to represent hero!

Player class

You should introduce a Player to represent what is particular to one player. From reading your code there is a pattern in the naming that help to find what belongs to the player. [red]Class, [blue]Class, [red]Name, [blue]Name do you see the pattern ? Here is what I used for the Player class :

public class Player {
 private String name;
 private Classes heroClass;
 private String heroName;
 private String color;
 public Player(String color) {
 this.color = color;
 }
 public String getName() {
 return name;
 }
 public void setName(String name) {
 this.name = name;
 }
 public Classes getHeroClass() {
 return heroClass;
 }
 public void setHeroClass(Classes heroClass) {
 this.heroClass = heroClass;
 }
 public String getHeroName() {
 return heroName;
 }
 public void setHeroName(String heroName) {
 this.heroName = heroName;
 }
 public String getColor() {
 return color;
 }
}

Refactoring

Let's start with choosing the alternate hero. What do you have is something like :

 String alternateHeroResponse = null;
 while(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 String[] alternateHeroResponseChoices = {
 "Choose one", 
 "Uther Lightbringer", 
 "Lady Liadrin"
 };
 alternateHeroResponse = (String) JOptionPane.showInputDialog(null, redName + ", which hero do you want to use?", "Red hero", JOptionPane.QUESTION_MESSAGE, null, alternateHeroResponseChoices, alternateHeroResponseChoices[0]);
 if(alternateHeroResponse == null || alternateHeroResponse == "Choose one") {
 System.err.println(redName + " did not pick a hero!");
 }
 else {
 redHero = alternateHeroResponse;
 }

What is common each time you want to make the user choose an alternate hero ? The choices of hero is always depending on the class you have, that's one thing. Which player is choosing will always change depending on who's turn is it to choose and that's it. So let's make a method with those two things as parameters!

private static String chooseAlternateHero(Player player, String[] heroNames) {
 String alternateHeroResponse = null;
 while (alternateHeroResponse == null) {
 alternateHeroResponse = (String) JOptionPane.showInputDialog(null,
 player.getName() + ", which hero do you want to use?", player.getColor() + " hero",
 JOptionPane.QUESTION_MESSAGE, null, heroNames, "Choose one");
 if (alternateHeroResponse == null) {
 System.err.println(player.getName() + " did not pick a hero!");
 }
 }
 return alternateHeroResponse;
}

---

Let's tackle the hero selection. What is common between what the red player and the blue player have to do ? The classes are the same for both of them so we should re-use our new Enum Classes for it. Since our Enum contains the possible heros we can easily use our new method previously refactored to make good use of that fact. The JOptionPane is always the same too expect for the message show to the user which have the name of the player and the color of the player. What you had was :

 String[] heroes = {
 "Choose one", 
 "Druid", 
 "Hunter", 
 "Mage", 
 "Paladin", 
 "Priest", 
 "Rogue", 
 "Shaman", 
 "Warlock", 
 "Warrior"
 };
 redClass = (String) JOptionPane.showInputDialog(null, redName + ", pick your class", "Red class", JOptionPane.QUESTION_MESSAGE, null, heroes, heroes[0]);

And what I have is :

private static Classes chooseHero(String name, String colorClass) {
 return (Classes) JOptionPane.showInputDialog(null, name + ", pick your class", colorClass + " class",
 JOptionPane.QUESTION_MESSAGE, null, Classes.values(), "Choose One");
}

Now we can re-use the method every time we want a player to choose a hero.

We can now isolate the fact that a player can choose a hero with our new method.

private static void pickHero(Player player) {
 while (player.getHeroClass() == null) {
 player.setHeroClass(chooseHero(player.getName(), player.getColor()));
 if (player.getHeroClass() == null) {
 System.err.println(player.getName() + " did not pick a class!");
 } else {
 if (player.getHeroClass().getHeroNames().length > 1) {
 player.setHeroName(chooseAlternateHero(player, player.getHeroClass().getHeroNames()));
 } else {
 player.setHeroName(player.getHeroClass().getHeroNames()[0]);
 }
 System.out.println(player.getName() + " has chosen " + player.getHeroClass().toString()
 + " and will play as " + player.getHeroName() + ".");
 }
 }
}

By using the parameter Player, we can now use this method for both player since everything we need will be in the object Player. See how I use both chooseHero (which should probably called chooseClasses now to think of it) and chooseAlternateHero with the value returned from the chosen class! Now your logic is independant from the concept you had hard-coded in your variables!

Here is the final class :

//A recreation of the game Hearthstone in Java
//Created 04/17/16 by Trevor B.
//Last updated 04/26/16
import java.util.Random;
import java.util.Scanner;
import javax.swing.JOptionPane;
public class Hearthstone {
 public static void main(String[] args) {
 boolean redTurn;
 boolean blueTurn; // for later
 /////////////////////////
 Scanner keyboard = new Scanner(System.in);
 Random randomizer = new Random(); // for later
 // Welcome message
 System.out.println("Hearthstone (Redux)\nBy Trevor B.\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
 // Assigning players
 Player red = new Player("Red");
 System.out.print("Red player, enter your name: ");
 red.setName(keyboard.nextLine());
 Player blue = new Player("Blue");
 System.out.print("Blue player, enter your name: ");
 blue.setName(keyboard.nextLine());
 // Picking hero
 pickHero(red);
 pickHero(blue);
 }
 private static Classes chooseHero(String name, String colorClass) {
 return (Classes) JOptionPane.showInputDialog(null, name + ", pick your class", colorClass + " class",
 JOptionPane.QUESTION_MESSAGE, null, Classes.values(), "Choose One");
 }
 private static String chooseAlternateHero(Player player, String[] heroNames) {
 String alternateHeroResponse = null;
 while (alternateHeroResponse == null) {
 alternateHeroResponse = (String) JOptionPane.showInputDialog(null,
 player.getName() + ", which hero do you want to use?", player.getColor() + " hero",
 JOptionPane.QUESTION_MESSAGE, null, heroNames, "Choose one");
 if (alternateHeroResponse == null) {
 System.err.println(player.getName() + " did not pick a hero!");
 }
 }
 return alternateHeroResponse;
 }
 private static void pickHero(Player player) {
 while (player.getHeroClass() == null) {
 player.setHeroClass(chooseHero(player.getName(), player.getColor()));
 if (player.getHeroClass() == null) {
 System.err.println(player.getName() + " did not pick a class!");
 } else {
 if (player.getHeroClass().getHeroNames().length > 1) {
 player.setHeroName(chooseAlternateHero(player, player.getHeroClass().getHeroNames()));
 } else {
 player.setHeroName(player.getHeroClass().getHeroNames()[0]);
 }
 System.out.println(player.getName() + " has chosen " + player.getHeroClass().toString()
 + " and will play as " + player.getHeroName() + ".");
 }
 }
 }
}

There is still improvement but it's a good start. What is missing in my code is that the String array for the hero names should be an Enum on it's own. I had this enum prepared, but I could not make the JOptionPane work with the hero name. I will give you the enum so that you can play with it as an exercise.

public enum Hero {
 Malfurion_Stormrage("Malfurion Stormrage"),
 Rexxar("Rexxar"), 
 Alleria_Windrunner("Alleria Windrunner"),
 Jaina_Proudmoor("Jaina Proudmoor"),
 Medivh("Medivh"),
 Khadgar("Khadgar"),
 Uther_Lightbringer("Uther Lightbringer"),
 Lady_Liadrin("Lady Liadrin"),
 Anduin_Wrynn("Anduin Wrynn"),
 Valeera_Sanguinar("Valeera Sanguinar"),
 Thrall("Thrall"),
 Gul_dan("Gul'dan"),
 Garrosh_Hellscream("Garrosh Hellscream"),
 Magni_Bronzebeard("Magni Bronzebeard");
 
 private String name;
 
 Hero(String name) {
 this.name = name;
 }
 public String getName() {
 return name;
 }
}

• Marc-Andre: BTW I removed the Choose one option since I find it more problematic than necessary. I could come up with a solution for this if you want.
• Arcturus: Thanks for taking the time to write this. It is my intention to have a Choose one option. Also, just to make sure, when I want to create a new method that both players will perform (let's call it makeDeck) it would be a method with the form public static Deck makeDeck(Player player) { }, correct? (Assume the Deck class will be created)
• Marc-Andre: @Eridan For the Choose one, I'll may try to hack something and help you with that. For the makeDeck method, well yeah something like would work. At some point, you will probably have a Game class that will manage some of those concepts. Making everything static is ok for the moment but it does not scale well.
• Arcturus: What would be a way to return the name of the class? (return "Druid" instead of "Malfurion Stormrage" for example)
• Arcturus: Ok, I'll make a chat when I'm more available.