Return to Revisions

Firstly, as a pointer, you can save yourself quite a lot of typing if you take the 2 load methods and extract what they have in common. Extracting methods is something you should always look for. Rule of thumb: if you type the same structure twice, you probably want to extract the common part into a method.

Here is an example of your method. There are a number of things you should improve in it but I'll list that later on. The reason extracting is important is that say you want to change how you handle exceptions, in your current setup you need to do it multiple times. You might also introduce small changes between the 2 methods (say in one case you catch an exception in the other you don't).

If you extract it to a method you update it once and it gets applied in the same way for all situations (which is usually what you want):

public String loadString(String propertyFileName, String key, String default) {
    String result = "";
    try{
        prop.load(new FileInputStream(propertyFileName));
        result = prop.getProperty(key);
    } catch (IOException e) {
        e.printStackTrace();
        // throw an exception or return default value
        // copying the code you had here before, though I would recommend not
        // doing main logic inside an exception. in particular as this exception
        // can be a number of things, not just filenotfound.
        prop.setProperty("Save_Point", "0");
        prop.store(new FileOutputStream(propertyFileName), null);

        try{
           prop.load(new FileInputStream(propertyFileName));
           result = prop.getProperty(key);
        }catch (IOException exe){
            exe.printStackTrace();
        }
    }
    return result;
}

Then you can use the loadGame and loadName like:

public String loadGame() {
    return loadString("config.prop", "Save_Point", "0");
}

public String loadName() {
    return loadString("config.prop", "Save_Name", "");
}

Furthermore, I would suggest you try to structure your questions / options / text in objects. If you expand it using the switch, you will get a monster of a switch statement which will be really hard to maintain.

You could do something like (assume there are getters/setters for these fields):

public class Room {
    protected int id;
    protected String description;

    protected List<Exit> exits;    
}

public class Exit {
    protected int leadsTo; // the room id
    protected String name;
}

These you could load from a file on disk (JSON format, property format or XML). You could check out jackson deserializer, though there are a number of options here.

Then in your loop you could do something like:

Map<int, Room> rooms = loadRoomsFromDisk(); // map of roomId -> Room
Room current = rooms.get(0); 
while (true) {
    // print out room description
    System.out.println(current.getDescription());
    // print out exits
    for (Exit exit : current.getExits()) {
        System.out.println(exit.getName());
    }

    // scan for line
    // if line corresponds to exit, go to exit. set current to new room.
}

This is oversimplified but to give you the idea. Now it would just be a matter of adding new room files and your gamearea would expand. You wouldn't need to update your main loop.

Keep in mind, this approach means that if you change your classes, the files on disk have to change. Initially it might be faster to create 1 levelloader class which just constructs a bunch of objects for you (a test level). Once that is working and stable (say you haven't changed the structure in a week or more) then you could consider writing it to disk/reading it from disk.

Now some general code review pointers:

1. In Java, start a method with lowerCaseFirst and start classes with UpperCaseFirst.

2. Never catch an exception without logging, unless you are absolutely sure that it is safe.

3. Try to avoid putting a lot of default logic in the exception part. In this specific case you have an IOException that could be a number of things. If there is a situation you want to catch (say, file doesn't exist on disk) then test for it in the try, and act on it, rather than trying out an operation and responding to the exception.

    File f = new File(propertyFile);
    if (!f.exists()) {
        // create the file
    }
   

Rather than:

    File f = new File(propertyFile);
    try {
        prop.load(new FileInputStream("config.prop"));
    } catch (FileNotFoundException e) {
        // create the file
    }

So far it looks good. I would sketch out on paper what the general flow is of your game. Designing levels can be tricky and it's easier to find out on paper what you want to build than to build stuff in code and after a lot of typing finding out you missed something. You probably will end up with some standard things like an inventory system, a player object, mobs, etc. Take it one step at a time and don't let yourself get overwhelmed by the things you want to put in.