Java can't store Object in arraylist

Question

Basically, I'm making a yahtzee game in MVC. It works fine, but When I try to save a session of a game, It doesn't save the game object containing and arraylist of players. This without stopping the execution which would certainly clear the arraylist of games. Basically, an instance of game has the following fields:

public String name;
public int numberOfPlayers;
public ArrayList<Player> players = new ArrayList<>();
int playerListIndex;
ArrayList<Boolean> checkable;
StandardRules yahtzeeRule = new StandardRules();
public int rounds;
public String date;

The Arraylist of players contains instances of the object Player which has it's own fields and gettters/setters.

I've managed to track the problem to not saving when I call the method:

public void saveGame(Game thisGame) {
    DB.saveGame(thisGame);
}

Which in turn calls the class:

public class DB {

private static ArrayList<Game> savedGames = new ArrayList<>();

/**
 * Saves the passed member into the database.
 * @param game, the member to be saved.
 */
public static void saveGame(Game game) {
    for (Game g : savedGames) {
        if (g.name.equals(game.name)) {
            savedGames.remove(g);
            savedGames.add(game);
        }
        else {
            savedGames.add(game);
        }
    }
}

The saveGame method basically checks the Arraylist of saved games if the name already exists, and if it does it removes the old one and adds the new one. If there is no game by that name, It just adds the game to the list. (I might Implement a MYSQL DB later, but for the sake of making sure the program works first, I'd like to use the arraylist to test things)

I'm under the impression that I need to have a static reference to the DB to ensure that I'm trying to access the same DB all the time and not mix up instances of a DB.

What am I doing wrong here?

looking at your code, i feel HashMap will be good than ArrayList. Each time don't have to iterate. — Sunny
– Sunny, Commented Jan 6, 2016 at 13:53

SomeJavaGuy · Accepted Answer · 2016-01-06 13:53:16Z

2

Try to follow the logic. You are looping over the array to save the game. If the array is empty, what would you expect to happen? If the Array would have six elements what would you expect to be in this list? In addition, you would most likely run into a ConcurentModificationException if this loop would ever get executed with a name that is already in the List. You might want to loop over the List and store a duplicate into a variable. But don´t do the saving and removing in the loop.

So in the end your problem is that you are looping over the empty list to save the Game, which in fact wont work because your List is empty and you will never reach the part where you add something to your List.

You are most likely out for such a solution to store the game in your List.

public static void saveGame(Game game) {
    Game dupGame = null;
    for (Game g : savedGames) {
        if (g.name.equals(game.name)) {
            dupGame = g;
        }
    }
    if (dupGame != null) {
        savedGames.remove(dupGame );
    }
    savedGames.add(game);
}

EDIT:

You could also make use of a Map, which would make finding duplicates and so on a bit faster and more easy.

public class DB {

    private static Map<String,Game> savedGames = new HashMap<>();

    /**
     * Saves the passed member into the database.
     * 
     * @param game
     *            , the member to be saved.
     */
    public static void saveGame(Game game) {
        savedGames.put(game.name.toLowerCase(), game);
    }
}

edited Jan 6, 2016 at 13:53

answered Jan 6, 2016 at 13:46

SomeJavaGuy

7,3872 gold badges24 silver badges33 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Danny Over a year ago

This solved my issue, I totally forgot about the concurrency aspect. Thanks man!

BValluri · Accepted Answer · 2016-01-06 13:59:22Z

You want to have these savedGames in memory right. There is no problem having them as static. The only problem you have is you are iterating through the savedGames list and also modifying it adding and removing the few games. This is wrong. It might give you concurrent update exception.

public class DB {

    private static ArrayList<Game> savedGames = new ArrayList<>();

    /**
     * Saves the passed member into the database.
     * @param game, the member to be saved.
     */
    public static void saveGame(Game game) {
          boolean exists=false;
          boolean removalObject=null;
          for (Game g : savedGames) {
            if (g.name.equals(game.name)) {
                exists=true;
                removalObject=g;
            }
        }
        savedGames.add(removalObject);
        savedGames.add(game);
    }
}

user4852164 · Accepted Answer · 2016-01-06 14:46:10Z

The first problem is , trying to add a game to the empty ArrayList will fail because the adding operation is done within the loop which will never run if the savedGames is empty. Also the remove(Object obj) method works as expected if the equals(Object obj) of the Object class is properly override in the Game class. Try this below... Override the equal() method in the Game Class:

@Override
public boolean equals(Object obj) {
    if(obj != null && obj instanceof Game){
        Game gameObj = (Game)obj;
        if(this.name.equals(gameObj.name)){
            return true;
        }
    }
  return false;
}

Do the saveGame() method as follows:

public static void saveGame(Game g){
    if(games.contains(g)){
        games.remove(g);
    }
    // Add the game after removing the existing 
    // or if never existed
        games.add(g);
}

HardCoded · Accepted Answer · 2016-01-06 13:47:52Z

0

You'r method can return a exception becuas you read from the ArrayList and add content to it in loop.

Try this. :)

for(Game g : savedGames) {
    if(g.name.equals(game.name)) {
        savedGames.remove(g);
        savedGames.add(0, game);
    } else {
        savedGames.add(0, game);
    }
}

answered Jan 6, 2016 at 13:47

HardCoded

1342 silver badges8 bronze badges

Collectives™ on Stack Overflow

Java can't store Object in arraylist

4 Answers 4

1 Comment

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related