HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMajor

Please Save My Name

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
savenameplease

Problem

I wanted to create a save game system for my city building game that did not require the player to input a name for the city. It was also important to allow infinite save games, so I could not just use save slots to solve the problem. With a bit of help, I devised a system that would allow the player to optionally enter a name, and if they didn't the game would determine a default name based on the existing save names.

At first I was just counting all of the existing worlds and then adding that number to "world" to get the name. However, I was concerned about the edge case where someone would, for example, make 5 worlds, then delete "world4", and then the game would suggest the name "world5" again, and if the player wasn't paying attention, their save would be overwritten.

When the user chooses to start a new game, a TextField appears that is populated by the correct string, which would be "world" followed by the next unused number.

String startName = this.getFirstUnusuedWorldName(this.getNumWorlds());
this.worldNameField = new TextField(startName, this.skin);


Here is the getNumWorlds() method:

private int getNumWorlds() {
    FileHandle[] files = Gdx.files.local("worlds/").list();
    int numDirectories = 0;
    for (FileHandle handle : files) {
        if (handle.isDirectory()) {
            numDirectories++;
        }
    }
    return numDirectories;
}


And here is the getFirstUnusuedWorldName method:

```
private String getFirstUnusuedWorldName(int numWorlds) {
FileHandle[] files = Gdx.files.local("worlds/").list();
if (files.length == 0) {
return "world1";
}

ArrayList possibleNames = new ArrayList();
for (int i = 1; i <= numWorlds + 1; i++) { //world names start with 1, need to search 1 past length
possibleNames.add("world" + String.valueOf(i));
}
for (String possibleName : possibleNames) {
boolean containsName = false;
for (FileHandle file : files) {
if (

Solution

You manage to do the right thing, but the way that you do it is a bit... what shall we say...? twisted? backwards? non-optimal!

Your approach:

  • Get list of all files in directory



  • Loop over a range and add the possible names to a list



  • Loop through the possible names and check if it matches an existing file



My comments to that:

  • Using the correct data structures would help greatly here. Adding the existing file names to a Set will make the lookup time \$O(1)\$.



  • Adding the names to a list is just completely unnecessary as you loop through that list directly afterwards.



  • Looping through a specific range is also unnecessary. Start loop from 1 and just don't stop until you have found a free spot.



  • This will also make the special-case files.length == 0 unnecessary.



Here's what we can end up with:

private String getFirstUnusuedWorldName() {
    FileHandle[] files = Gdx.files.local("worlds/").list();
    Set fileNames = new HashSet();
    for (FileHandle handle : files) {
        fileNames.add(handle.getName());
    }

    for (int i = 1; ; i++) {
        String name = "world" + i;
        if (!fileNames.contains(name)) {
            return name;
        }
    }
}


Note the lack of a stopping condition in the for-loop (You don't need one as sooner or later, there will be an empty spot).

Also note that you no longer need the int numWorlds parameter.

Code Snippets

private String getFirstUnusuedWorldName() {
    FileHandle[] files = Gdx.files.local("worlds/").list();
    Set<String> fileNames = new HashSet<String>();
    for (FileHandle handle : files) {
        fileNames.add(handle.getName());
    }

    for (int i = 1; ; i++) {
        String name = "world" + i;
        if (!fileNames.contains(name)) {
            return name;
        }
    }
}

Context

StackExchange Code Review Q#96285, answer score: 27

Revisions (0)

No revisions yet.