global array of objects that any class can access?

[Jesse Matty]

public  class  Game  {
	public  int playerTurn=0;
	public   Player [] gamePlayers;

	public   Map  gameMaps;
	public  JFrame gameWindow= new JFrame("Black Tower");
	public   int numberOfPlayers;
	
	public  Game() {		
	}
	
	public static void main(String[] args) {
		new MainWindow();
	}

I have public array of players that I have declared to keep track of all the player information during the game. This array is  given spaces for 4 players  in the mainwindow class that extends game and then initialized  with a for loop once the number of players is selected. All classes that need to get the player information extend game or extend another class that extends game. My problem is though after the players are created when another class tries to access the variables they get null pointer exceptions. I believe this is because every time a new  class Game is created the gamePlayers array points to null right? So how do I have  a global array of player objects that any class can access?


[HENRY: Fixed Code format]

[Campbell Ritchie]

Jesse Matty wrote:. . . So how do I have  a global array of player objects that any class can access?

Why do you want a global array? Global variables are error‑prone. Remember that arrays are always mutable; you can always say
MyArray[123] = something;
If you have nulls in your array, you can avoid that by creating the array via an array initialiser, or otherwise populating the array.

[Jesse Matty]

making the array static solved my problem

[Campbell Ritchie]

Jesse Matty wrote:making the array static solved my problem

I am 99% sure it didn't solve the problem. It probably added several new problems.

[Jesse Matty]

what kind of problems could it have added? If you have 4 players in a game  with numerous classes that need to access information on the player class  in a turn based game depending on whose turn it is aka player object number how else would you do it other than a  global array of players?

[Campbell Ritchie]

If you make the array static, then you are only ever allowing for one game; if you start a second game, then you will have those players playing the first game.
You can easily make the array private to the game class; there are ways to make it available to other code which has business knowing the players via getXXX methods. You can even do it in such a way that the original array cannot be changed outside the game object. There are lots of ways to achieve what you want without global variables.

[Stephan van Hulst]

Classes that need information from the Game do so by interacting with the instance of the Game through methods defined in Game. They have access to the Game because you pass the Game to those classes in their constructors or methods. Don't interact with a class' fields directly, and don't extend a class just to get access to its members.

Let's say that you have some sort of user interface that needs to display information from the game. It can access the information like this:

final class GameGUI {

  private Game game;

  void setGame(Game game) {
    this.game = game;
    update();
  }

  private void update() {
    // Update the user interface.
    // You can get information from the game by calling methods like game.getPlayers()
  }
}

[Jesse Matty]

so even if I run two copies of the same program jar ( make a duplicate jar file using the os) file both will have the same player objects ? I wasn't intending on having more than one game being played at any point and time., or allowing anyone else to be able to that  as it doesn't make much sense., or ever needing to make different game object.

[Stephan van Hulst]

Jesse Matty wrote:what kind of problems could it have added?

It's very easy to introduce bugs in these kinds of programs, because objects can't guarantee that their fields are in a valid state, because they can be accessed by anyone, from anywhere.

[Stephan van Hulst]

Jesse Matty wrote:so even if I run two copies of the same program jar ( make a duplicate jar file using the os) file both will have the same player objects?

No. Different program executions will have separate state.

I wasn't intending on having more than one game being played at any point and time., or allowing anyone else to be able to that  as it doesn't make much sense., or ever needing to make different game object.

That doesn't matter. Even if you only ever have one instance of a type, it's very bad to make it available globally if it doesn't represent a constant value. It becomes incredibly easy to make mistakes that will cause bugs that are hard to track down.

[Jesse Matty]

Stephan van Hulst wrote:Classes that need information from the Game do so by interacting with the instance of the Game through methods defined in Game. They have access to the Game because you pass the Game to those classes in their constructors or methods. Don't interact with a class' fields directly, and don't extend a class just to get access to its members.

Let's say that you have some sort of user interface that needs to display information from the game. It can access the information like this:

final class GameGUI {

  private Game game;

  void setGame(Game game) {
    this.game = game;
    update();
  }

  private void update() {
    // Update the user interface.
    // You can get information from the game by calling methods like game.getPlayers()
  }
}

but they don't need any information from game just  the  player class. I know about making methods to access the information in  a class but java is objected oriented  and was made resemble real world interactions and in the real world if a bazar owner wants to talk to a  Army leader he doesn't need to talk to anyone  else.  It would be easy if I only  had one player to use the player class getters and setters  and / or other methods to modify the player class but there could be up to 4 of them who are playing in front of one computer.  So other classes that interact with player need to know which player they are interacting with and there need to be a away to store all 4 players information and access / modify it;  and  to make even more getters,  setters, and other methods in the game class that just call the player class  methods seems kind of redundant.

[Stephan van Hulst]

This will be easier if you can give an example of a class that needs access to a single player. We can then show you a small example of how it's done, without having to use global data.

[Campbell Ritchie]

Jesse Matty wrote:so even if I run two copies of the same program jar . . . file both will have the same player objects ? . . .

If you make the array static, yes. At least within the same JVM.
The bit about multiple copies of the same .jar is a red herring. If you run two copies of the same thing in two JVMs, what difference will there be if you start with the same .jar file or an identical copy?

[Jesse Matty]

Stephan van Hulst wrote:This will be easier if you can give an example of a class that needs access to a single player. We can then show you a small example of how it's done, without having to use global data.

I know how to do it without global data it is just A LOT more work, and seems unnecessary but since this program is going to part a portfolio to help land me a job if making more methods to pass information will look better then make them I shall!. But also on side question what is a  red herring other than  a smoked fish that tastes like ham?

[Bear Bibeault]

Red herring

[Campbell Ritchie]

Jesse Matty wrote:. . . part a portfolio to help land me a job

If you want a job, don't let them see global variables.

. . . what is a  red herring . . .

Something which sends you on a wild goose chase.

Bear has already given you a more helpful link.

[Jesse Matty]

is this a an acceptable  solution? or should I still write methods in the  Game class to call the over 200 public methods the player class has?

here is my main class

public  class  Game  {
	 public   int playerTurn;
	  public    Player [] gamePlayers;

	public   Map  gamemaps;
	public  JFrame gamewindow= new JFrame("Black Tower");
	 public   int numberOfPlayers;
	







	public  Game() {	
		
	}
	
	
	public static void main(String[] args) {

		new MainWindow();
		
		
		

	
	
	
	}
public int getPlayerTurn(){
	return playerTurn;
	
	
}
	
public void playerTurnChange(){
	


}

	 public Player getPlayer (int number){
		
		
		return gamePlayers[number];
	}
		

then in any class that needs to modify the player I write the other classes still extend game and other classes still extend them .

protected int  playerTurn=getPlayerTurn();
	protected Player player=getPlayer(playerTurn);

[Jesse Matty]

Also I just  made all fields of  game class other than numberofplayers private

[Jesse Matty]

my solution was not acceptable  still null pointers

[Campbell Ritchie]

Jesse Matty wrote:is this a an acceptable  solution? . . .

public  class  Game  {
	 public   int playerTurn;
// ...

. . .

No.

Don't make variables coderanch. Unless you have a special arrangement with colleagues for special circumstances, make all variables private. Why are you giving things protected access in the second code block?

Your bits of code look as thought you were guessing what to do next. Don't. You can make 1000000 different guesses and there is a good chance a few will be correct, or you can plan the application beforehand and it will be right quickly. I think you shou‍ld turn your computer off and write down on paper what the game is supposed to be about.

[Jesse Matty]

I know what the applacation is supposed  be about  now I'm redoing parts of  it  because my design stank. I extended every class to some other class.  I have written over 20,000 lines of code  I'm re doing some of it; if I had to rewrite it all I would just make something else instead.

[Jesse Matty]

Stephan van Hulst wrote:Classes that need information from the Game do so by interacting with the instance of the Game through methods defined in Game. They have access to the Game because you pass the Game to those classes in their constructors or methods. Don't interact with a class' fields directly, and don't extend a class just to get access to its members.

Let's say that you have some sort of user interface that needs to display information from the game. It can access the information like this:

final class GameGUI {

  private Game game;

  void setGame(Game game) {
    this.game = game;
    update();
  }

  private void update() {
    // Update the user interface.
    // You can get information from the game by calling methods like game.getPlayers()
  }
}

if  I do

Game game;
JFrame window =game.getJframe();

how do I  access the same instance of game every time?

[Stephan van Hulst]

Game shouldn't have a JFrame. Game is part of your business logic, and a JFrame is part of the view.

Access the same instance of Game every time from where?

[Jesse Matty]

I have solved  my problem using the singleton design  method I think bay making a class that holds all of the information such as game map the array of  players  and making a static reference to that single class

public final class GamePlayers {
	 private Player [] gamePlayers = new Player [4];
	private int  playerTurn;
	private int numberOfPlayers;
	private JFrame window;
	private Map gameMap;
	private static  GamePlayers gameData;
	
	private GamePlayers(){
		
	}
	public static GamePlayers getGamePlayers (){
		if (gameData==null){
			gameData=new GamePlayers();
		}
		
		
		return gameData;
	}
	
	
	
	
	
	public void makePlayers( int number){
		
		for (int count=0;  count<number; count++){
			gamePlayers[count]= new Player();
		}
		numberOfPlayers=number;
	}
		
		public Player getPlayer (int number){
			
			 
			  return gamePlayers[number];
			  
			
		 }
		public void setPlayer (int number, Player player){
			
			 gamePlayers[number]=player;
			 
			  
			
		 }
		
		
		
		
		
		
		
		
		public void playerTurnChange(){
			playerTurn++;
			if (playerTurn>numberOfPlayers){
				playerTurn=0;
			}


		}
		public int getPlayerTurn(){
			
			return playerTurn;
		}
		
		
		
		public int getNumberOfplayers(){
			return numberOfPlayers;
		}

		public JFrame getWindow() {
			return window;
		}

		public void setWindow(JFrame window) {
			this.window = window;
		}

		public Map getGameMap() {
			return gameMap;
		}

		public void setGamemap(Map gamemap) {
			this.gamemap = gameMap;
		}
	public void makeMap(){
		gamemap=new Map();
		gamemap.generateMap();
		
	}
		
	
	

[Campbell Ritchie]

Why on earth do you want a singleton?
That is not the correct way to implement a singleton even if you do want one. If you must, then do it like this:-

public class Singleton
{
   private static final Singleton INSTANCE = new Singleton(...);

   private Singleton(...)
   {
      // some code or other.
   }

   public static Singleton getInstance()
   {
      return INSTANCE;
   }
   // etc
}

[Junilu Lacar]

Jesse Matty wrote: I know what the applacation is supposed  be about  now I'm redoing parts of  it  because my design stank. I extended every class to some other class.  I have written over 20,000 lines of code  I'm re doing some of it; if I had to rewrite it all I would just make something else instead.

That's liable to scare employers away rather them attract them.

[Stephan van Hulst]

Well, you replaced a globally accessible array with a globally accessibly mutable object. That's not really an improvement.

Every class needs the bare minimum of fields that they require to implement their behavior, without having to access static fields from elsewhere.

Let's take a simple game of Tic Tac Toe as an example. Here's how such a program could roughly look like:

package tictactoe.model;

public enum Mark {

  CROSS, NOUGHT;

}

package tictactoe.model;

public final class Board {

  private final Mark[][] marks;

  Board();

  public Optional<Mark> getMark(int x, int y);

  void placeMark(int x, int y, Mark mark);

}

package tictactoe.model;

public final class Game {

  private final Board board;
  private final Map<Mark, Player> players;

  private Mark currentMark;

  public Game(Player player1, Player player2);

  public Board board();

  public Mark getCurrentMark();

  public Player getCurrentPlayer();

  public Player getPlayer(Mark mark);

  public boolean isGameOver();

  public boolean isDraw();

  public Optional<Player> getWinner();

  public void playTurn();  

}

package tictactoe.model;

public abstract class Player {

  private final String name;

  protected Player(String name);

  public final String name();

  protected abstract Move getNextMove(Board board, Mark markToPlace);

}

package tictactoe.view;

final class GamePanel extends JPanel {

  private Game game;

  GamePanel();

  void setGame(Game game);

}

package tictactoe.view;

final class Application {

  private final JFrame frame;
  private final GamePanel gamePanel;

  Application();

  public static void main(String... args);

}

You can see here that every class has exactly the fields they require in order to implement their methods. These fields are mostly set in the constructor.

An Application creates a JFrame and a GamePanel. When you click on a menu item for a new game, it shows a prompt to enter the players' names, and then it constructs a new Game which it sets on the GamePanel. Whenever the user clicks on one of the buttons of the GamePanel, the GamePanel instructs the Game to play a turn. The Game asks the current Player for a Move, and if the Move is valid, it places the current Mark on the Board in the corresponding location. Afterwards, GamePanel can update its entire user interface because it can access all the information it needs through the game field.

[Junilu Lacar]

Very nice example, Stephan!  I'd give a cow to see code like that more often around here.

[Jesse Matty]

My game is a more advanced copy of the board game dark tower (https://boardgamegeek.com/boardgame/30/dark-tower). The player moves to various square and things happen to him. He could get into battle, get lost find food,  find gold, or buy / sell at the bazar or recrute men at town or castle  visit a tomb ruin or tower He has gold grain  water meat fruit oil, tents, canoes, wands armor , horses,  potions ect,  and array lists  for the various classes  of men he has in his army ( possibly should have only made one  soilder class and one array list but as it stands there are five ). The player is basically  a database with many functions to do things and interact with other classes.  There needs to be 1- 4 player objects modifiable by  most others classes with a place to store them,    I saw no other way to do this other than some kind of global something be it variable or class.



here are my player class fields

	
	GamePlayers players=GamePlayers.getGamePlayers(); // gets the game data for the map and players
	
		private int health; // player health for single player
	private int experience;
	private int strength;
	private int gold; // amount of money
	private int water; // water amount
	private int meat; // food amounts
	private int grain;
	private int fruit;
	private int taintedMeat; // amounts of rotten fruit and player has that make men sick and cause damage the he found..
	private int taintedFruit; // amounts of tainted fruit he found
	private int totalarrows; // total arrows for player
	private int headCount; // total number of men player has
	private int numberofLights;
	private Map gamemaps; // holds the game map to add the player location to it.
	private Player player; // this instance of one of the players
	int playerTurn; // the player number turn
	private int [] wands= new int [16]; // number of each kind of wand the player has

	private int [] poitions = new int  [11]; //player potions each array value is the number of potitions that of that kind number
	private int dead; // how many men died
	
		private int starve; // how many starved due to lack of food
	private int numberoftents; // number of tents
	private int numberofcanoes; // number of canoes player has

	private boolean menstarving; // are men starving?
	private int upgrade=0; // weapoon upgrade number
	private boolean [] mapKeys = new boolean [5]; // keys for each map
	private boolean hasscout; // does the player have a scout?
	private int skeltonKeyNumber; // how many skelton keys the player has
	private int [] arrows = new int [3]; // numbers of various kinds of arrows
	private int [] armor = new int [12]; // numbers of each kind of armor
	private int [] weapons = new int [33]; // numbers of each kind of weapon
	
	private ArrayList <Axemen>  playerAxemen = new ArrayList<Axemen>(); //players army of men and horses axemen
	private ArrayList <Bowman>  playerBowman = new ArrayList<Bowman>();// Archers
	private ArrayList <Knight>  playerKnight = new ArrayList<Knight>(); // Knights
	private ArrayList <Horse>  playerHorse = new ArrayList<Horse>(); //Horses
	private ArrayList <Mage>  playerMage =new ArrayList<Mage>(); //Mages
	private ArrayList <Swordsmen>  playerSwordsmen = new ArrayList<Swordsmen>(); // swordsmen
	private ArrayList <Scout> playerScout =new ArrayList<Scout>(); // the player scouts
	private JFrame message= new JFrame(); //JFrame for popup messages
	private int playerAxemenNumber; //total number of axemen
	private int playerKnightNumber; // total number of knights
	private int playerHorseNumber; // total horses
	private int playerSwordsmenNumber; //total swordsmen
	private int playerBowmanNumber; // total number of bowman aka archers
	private int playerMageNumber; // total mages
	private int playerScoutNumber; // total scouts
	private double maxWeight; // the maximum weight a player can currently carry
	private double playerWeight;// the current weight of player goods
	private double newWeight; // how much weight was added
	private int oil; //players amount of oil
	private int locationx;  //where player is on map
	private int locationy;
	private int oldlocationx;
	private int oldlocationy; //where player was on map used when he tries to move
	private boolean playerhasscout;
	private int mapnumber; // the map the player is on
	RandomNumbers value = new RandomNumbers(); // random number class maker
	private int playersaddles; // how many saddles the player has
	
	

here are my player class functions some private some public

addArmor(int, int)
addArrows(int, int)
addAxe(int, int)
addBow(int, int)
addCanoe(int)
addFruit(int)
addGold(int)
addGrain(int)
addLight()
addLight(int)
addMap()
addMapKey(int)
addMeat(int)
addOil(int)
addPotion(int, int)
addSaddles(int)
addSkeltonKey(int)
addTaintedFruit(int)
addTaintedMeat(int)
addTent(int)
addWands(int, int)
addWater(int)
addWeapon(int, int)
axemenArmorCheck(int)
axemenBattleDamage(int, int, int)
axemenDamage(int, int)
axemenEat()
axemenWeaponCheck(int)
bowmanAddArrows(int, int)
bowmanAddFirearrows(int, int)
bowmanAddPosionarrows(int, int)
bowmanArmorCheck(int)
bowmanBattleDamage(int, int, int)
bowmanBowCheck(int)
bowmanDamage(int, int)
bowmanEat()
bowmanUseArrows(int, int)
bowmanUseFirearrows(int, int)
bowmanuseposionarrows(int, int)
carryExtrAarmor(int, double, int)
carryExtraWeapon(double, int, int)
carryFruit(double, int)
carryGold(double, int)
carryGrain(double, int)
carrymeat(double, int)
carryOil(double, int)
carryWater(double, int)
changeMageWand(int, int)
fillWater()
fullAxemenHealth(int)
fullBowmanHealth(int)
fullHorseHealth(int)
fullKnightHealth(int)
fullMageHealth(int)
fullScouthHealth(int)
fullSwordsmenHealth(int)
getArmor()
getArrows()
getAxemenArmor(int)
getAxemenWeapon(int)
getBowmanArmor(int)
getBowmanWeapon(int)
getDead()
getExperience()
getFruit()
getGold()
getGrain()
getHeadCount()
getHealth()
getKnightArmor(int)
getKnightWeapon(int)
getLocationx()
getLocationy()
getMageClass(int)
getMageWand(int)
getMapnumber()
getMaxWeight()
getMeat()
getNewWeight()
getNumberofcanoes()
getNumberoftents()
getOil()
getOldlocationx()
getOldlocationy()
getPlayerAxemen()
getPlayeraxemennumber()
getPlayerBowman()
getPlayerbowmannumber()
getPlayerHorse()
getPlayerhorsenumber()
getPlayerKnight()
getPlayerknightnumber()
getPlayerMage()
getPlayermagenumber()
getPlayersaddles()
getPlayerScout()
getPlayerscoutnumber()
getPlayerSwordsmen()
getPlayerswordsmennumber()
getPlayerWeight()
getPoitions()
getSkeltonKeyNumber()
getStarve()
getStrength()
getSwordsmenArmor(int)
getSwordsmenWeapon(int)
getTotalarrows()
getWands()
getWater()
getWeapons()
horseCarryCanoe(double, int)
horseCarryTent(double, int)
horseDamage(int, int)
horseEat()
horseStarve(int)
isHasscout()
isMenstarving()
isPlayerhasscout()
knightArmorCheck(int)
knightBattleDamage(int, int, int)
knightDamage(int, int)
knightEat()
knightWeaponCheck(int)
loseFruit(double, int)
loseGold(double, int)
loseGrain(double, int)
loseMeat(double, int)
loseOil(double, int)
loseWater(double, int)
mageBattleDamage(int, int)
mageDamage(int, int)
mageEat()
mageWandCheck(int)
mageWandUpgrade(int, int)
mendDamage(int)
menDead()
menRemoveObject(int)
menUseLights()
menWithoutLightDamage(int)
mountKnight(int, int)
newAxemen(Axemen)
newBowman(Bowman)
newHorse(int)
newKnight(Knight)
newMage(Mage)
newScout()
newSwordsmen(Swordsmen)
playerCrossRiver()
playerEndTurn()
playerEndTurnLost()
playerStartTurn()
playerTurn()
removeArmor(int, int)
removeCanoe(int)
removeExtraWeapon(double, int, int)
removeFruit(int)
removeGold(int)
removeGrain(int)
removeMeat(int)
removeOil(int)
removeTent(int)
removeWater(int)
removeWeapon(int, int)
scoutDamage(int)
scoutEat()
scoutHasMap()
setAxemenFighting(int, boolean)
setBowmanFighting(int, boolean)
setDead(int)
setExperience(int)
setFruit(int)
setGold(int)
setGrain(int)
setHasscout(boolean)
setHealth(int)
setKnightFighting(int, boolean)
setLocationx(int)
setLocationy(int)
setMageFighting(int, boolean)
setMapnumber(int)
setMaxWeight(int)
setMeat(int)
setMenstarving(boolean)
setNewWeight(int)
setNumberoftents(int)
setOil(int)
setPlayerAxemen(ArrayList<Axemen>)
setPlayeraxemennumber(int)
setPlayerBowman(ArrayList<Bowman>)
setPlayerbowmannumber(int)
setPlayerhasscout(boolean)
setPlayerHorse(ArrayList<Horse>)
setPlayerhorsenumber(int)
setPlayerKnight(ArrayList<Knight>)
setPlayerknightnumber(int)
setPlayerMage(ArrayList<Mage>)
setPlayermagenumber(int)
setPlayerScout(ArrayList<Scout>)
setPlayerscoutnumber(int)
setPlayerSwordsmen(ArrayList<Swordsmen>)
setPlayerswordsmennumber(int)
setPlayerWeight(double)
setPoitions(int[])
setSkeltonKeyNumber(int)
setStarve(int)
setStrength(int)
setSwordsmenFighting(int, boolean)
setWater(int)
sheildAttackAxemen(int, int, int, int, int)
sheildAttackBowman(int, int, int, int, int)
sheildAttackKnight(int, int, int, int, int)
sheildAttackSwordsmen(int, int, int, int, int)
swordsmenArmorCheck(int)
swordsmenBattleDamage(int, int, int)
swordsmenDamage(int, int)
swordsmenEat()
swordsmenWeaponCheck(int)
upgradeAxemenExperience(int)
upgradeAxemenIQ(int)
upgradeAxemenSpeed(int)
upgradeaxemenstrength(int)
upgradeAxemenWeapon(int, int)
upgradeAxemwnArmor(int, int)
upgradeBowmanArmor(int, int)
upgradebowmanexperience(int)
upgradeBowmanIQ(int)
upgradeBowmanSpeed(int)
upgradeBowmanStrength(int)
upgradeBowmanWeapon(int, int)
upgradehorsespeed(int)
upgradeKnightArmor(int, int)
upgradeKnightExperience(int)
upgradeKnightIQ(int)
upgradeKnightSpeed(int)
upgradeKnightStrength(int)
upgradeKnightWeapon(int, int)
upgradeMageExperience(int)
upgradeMageIQ(int)
upgradeMageMagic(int)
upgradeMageSpeed(int)
upgradeMageStrength(int)
upgradeScoutExperience(int)
upgradeScoutIQ(int)
upgradeSwordsmenArmor(int, int)
upgradeSwordsmenExperience(int)
upgradeSwordsmenIQ(int)
upgradeSwordsmenSpeed(int)
upgradeSwordsmenStrength(int)
upgradeSwordsmenWeapon(int, int)
useMagePower(int, int)
useSkeltonKey(int)

here are the classes that interact with player

LandSquare // the map tiles of land anything can occur from traps to finding things to battle
Bazar // a buy / sell shop
Tomb // a tomb where he can find stuff or battle with enemies can occur
Ruin // like a tomb again probably a redundant class
Tower // where the final battle happens for each of the five maps
RecurteMen // where the player can recrute new soliders of various kinds.
Battle // the fight class for player and enemies
ViewArmy // where can see his men
Inventory // shows what the player has
UpgradeMen // where after viewing his men he can upgrade them
Citidel // free food gold and men for  a weak player.
and of course the axemen bowmen, swordsmen knight, mage horse  and scout classes interact with it to.

[Junilu Lacar]

This group of methods and other method groups with similar name patterns are a code smell.
upgradeAxemenExperience(int) upgradeAxemenIQ(int) upgradeAxemenSpeed(int) upgradeaxemenstrength(int) upgradeAxemenWeapon(int, int) upgradeAxemwnArmor(int, int) upgradeBowmanArmor(int, int) upgradebowmanexperience(int) upgradeBowmanIQ(int) upgradeBowmanSpeed(int) upgradeBowmanStrength(int) upgradeBowmanWeapon(int, int) upgradehorsespeed(int) upgradeKnightArmor(int, int) upgradeKnightExperience(int) upgradeKnightIQ(int) upgradeKnightSpeed(int) upgradeKnightStrength(int) upgradeKnightWeapon(int, int) upgradeMageExperience(int) upgradeMageIQ(int) upgradeMageMagic(int) upgradeMageSpeed(int) upgradeMageStrength(int) upgradeScoutExperience(int) upgradeScoutIQ(int) upgradeSwordsmenArmor(int, int) upgradeSwordsmenExperience(int) upgradeSwordsmenIQ(int) upgradeSwordsmenSpeed(int) upgradeSwordsmenStrength(int) upgradeSwordsmenWeapon(int, int)
All those things after the "upgrade" could be classes of themselves. These could probably be boiled down to one polymorphic method:
upgrade(UpgradableCapability);
where UpgradableCapability is an interface or abstract class. Interface might be the better choice here. The upgrade method can have a variatic parameter list:

public void upgrade(UpgradableCapability capability, int... whateverTheseThingsAre)

[Stephan van Hulst]

Even if you need all those separate classes to access the different players, you don't need the players to be globally accessible to do it. Either you invert control, or you pass the player to a method of the class that needs to operate on a player.

However, look at these classes: LandSquare, Bazar, Tomb, Ruin, Tower... They look like places a Player can visit. If that doesn't scream visitor pattern, than I don't know what does.

public interface Visitable {

  void visitBy(Player visitor);

}

final class Tower implements Visitable {

  private final King king;

  @Override
  public void visitBy(Player visitor) {
    Battle battle = new Battle(visitor, king);
    // Do other stuff with the battle, visitor and king.
  }
}

public final class GameMap {

  private final Map<Location, Visitable> sites;

  public void visit(Location location, Player player) {
    sites.get(location).visitBy(player);
  }
}

Next, your player seems to have a lot of properties that do the same thing. Why not make a class that can count the amount a player has of a certain property type?

public interface Property<T> {

  public PropertyType type();

  public T value();

}

public interface MutableProperty<T> extends Property<T> {

  public void setValue(T newValue);

  public default Property<T> asReadOnlyProperty() {
    return new ReadOnlyProperty(this);
  }
}

public final class IntProperty extends AbstractProperty<Integer> implements MutableProperty<Integer> {

  private int value;

  public IntProperty(PropertyType type);

  public IntProperty(PropertyType type, int initialValue);

  public IntProperty(PropertyType type, int initialValue, int minimum, int maximum);

  @Override
  public Integer value();

  @Override
  public void setValue(Integer newValue);

  public int increase(int amount);

  public int decrease(int amount);

  public boolean isMinimal();

  public boolean isMaximal();

}

A Battle class could then do something like this:

void doBattle() {
  // calculate damage and stuff

  player.getMutableInt(PropertyType.HEALTH).decrease(damage);

  // do other stuff
}

[Jesse Matty]

Stephan van Hulst wrote:Even if you need all those separate classes to access the different players, you don't need the players to be globally accessible to do it. Either you invert control, or you pass the player to a method of the class that needs to operate on a player.

However, look at these classes: LandSquare, Bazar, Tomb, Ruin, Tower... They look like places a Player can visit. If that doesn't scream visitor pattern, than I don't know what does.

...

I got you on the visitable interface. But why another class to store player objects? and why to interfaces to go along with it? Also what does T mean?

[Stephan van Hulst]

Jesse Matty wrote:I got you on the visitable interface. But why another class to store player objects?

If you mean player's properties, it's because it removes the need to add both a setter and a getter for each property. For instance, your Player has getFruit(), setFruit(), getGold() and setGold(). The getters do almost exactly the same, except they're for different properties, and the setters do exactly the same, except for different properties. Now, you can just declare en enum PropertyType which succinctly lists all the properties a Player can have, such as FRUIT, GOLD and HEALTH. Another advantage is that this approach allows other classes (such as your game display) to inspect the properties of your Player, without having to hard code all properties. Without the property class:

package view;

final class PlayerInfoPanel extends JPanel {

  private final DefaultTableModel playerInfoTable;

  private Player player;

   ...

     playerInfoTable.addRow("Fruit",  player.getFruit());
     playerInfoTable.addRow("Gold",   player.getGold());
     playerInfoTable.addRow("Health", player.getHealth());
     // etc...

   ...
}

With the property class:

package view;

final class PlayerInfoPanel extends JPanel {

  private final DefaultTableModel playerInfoTable;

  private Player player;

   ...

     for (Property<?> property: player.getAllProperties()) {
       playerInfoTable.addRow(property.type(), property.value());
     }

   ...
}

why to interfaces to go along with it?

Because you want encapsulation and separation of concerns. A class that only needs to display generic properties does not need to know how to edit the property, nor what the underlying data type of the property is. We can see that PlayerInfoPanel can work just fine with Property, knowing only how to get the type() and the value(). One of your game elements might have to edit the player though (such as when the player is in a battle), and needs a MutableProperty<SomeSpecificType> to update it.

Also what does T mean?

T is a generic type parameter. Property<T> essentially says: I am a property of some type that you can determine later. For instance, your Player can have a property of a completely different type than int:

package model.property;

public class SimpleProperty<T> extends AbstractProperty<T> implements MutableProperty<T> {
  
  protected final boolean nullsAllowed;

  protected T value;

  public SimpleProperty(PropertyType type, T initialValue, boolean nullsAllowed) {
    super(type);

    if (initialValue == null && !nullsAllowed)
      throw new IllegalArgumentException("Initial value was null, but null is not allowed.");

    this.nullsAllowed = nullsAllowed;
    this.value = initialValue;
  }

  public SimpleProperty(PropertyType type, T initialValue) {
    this(type, initialValue, false);
  }

  public SimpleProperty(PropertyType type) {
    this(type, null, true);
  }

  @Override
  public T value() {
    return value;
  }

  @Override
  public void setValue(T newValue) {
    if (newValue == null && !nullsAllowed)
      throw new IllegalArgumentException("New value was null, but null is not allowed.");

    this.value = newValue;
  }
}

package model.property;

public final class LocationProperty extends SimpleProperty<Location> {
  
  public LocationProperty(PropertyType type, Location initialValue, boolean nullsAllowed) {
    super(type, initialValue, nullsAllowed);
  }

  public LocationProperty(PropertyType type, Location initialValue) {
    super(type, location);
  }

  public LocationProperty(PropertyType type, int x, int y) {
    super(type, new Location(x,y));
  }

  public LocationProperty(PropertyType type) {
    super(type);
  }

  public Optional<Integer> x() {
    return Optional.ofNullable(value).map(Location::x);
  }

  public Optional<Integer> y() {
    return Optional.ofNullable(value).map(Location::y);
  }

  public void setX(int x) {
    if (value == null)
      throw new IllegalStateException("The current location is null.");

    value = new Location(x, value.y());
  }

  public void setY(int y) {
    if (value == null)
      throw new IllegalStateException("The current location is null.");

    value = new Location(value.x(), y);
  }
}

Now, because we have these different interfaces, we can expose mutable properties to classes within the same package, and we can expose read-only properties to classes outside Player's package. This ensures that you don't accidentally corrupt your data from classes outside of your model.

package model;

import java.util.*;
import java.util.stream.*;

import model.property.*;

import static java.util.function.Function.*;
import static java.util.stream.Collectors.*;

import static model.property.PropertyType.*;

public final class Player {

  private final LocationProperty location;
  private final Map<PropertyType, IntProperty> intProperties;

  public Player(int x, int y, int health) {
    this.location = new LocationProperty(LOCATION, x,y);

    this.intProperties = Stream.of(
      new IntProperty(FRUIT,  0),
      new IntProperty(GOLD,   0),
      new IntProperty(HEALTH, health)
    ).collect(toMap(IntProperty::type, identity()));
  }

  MutableProperty<Location> getMutableLocation() {
    return location;
  }

  MutableProperty<Integer> getMutableInt(PropertyType type) {
    IntProperty property = intProperties.get(type);
    if (property == null)
      throw new IllegalArgumentException(String.format("The player does not contain a %s integer property.", type));

    return property;
  }

  public Property<Location> location() {
    return location.asReadOnlyProperty();
  }

  public Property<Integer> getInt(PropertyType type) {
    return getMutableInt(type).asReadOnlyProperty();
  }
}

A final advantage is that you can add listeners to your properties, so classes can get notified when they change.