5
\$\begingroup\$

I have recently started working on a server software in Java and wanted to add a configuration option to the program. I have gone through a few revisions of this class because I never liked what I came up with. I finally made something that I'm somewhat happy about and it works better than past versions. I'm just curious if anything can be done better or if this class should be rewritten again in a different?

FYI: some methods are not done yet. I'm not worried about those methods, I'm asking about what is there so far.

import java.io.File;
import java.util.HashMap;
import java.util.Map;

import org.yaml.snakeyaml.DumperOptions;
import org.yaml.snakeyaml.Yaml;

public class Config
{
public static final String SERVER_NAME = "serverName";
public static final String MOTD = "motd";
public static final String SERVER_PORT = "serverPort";

private static final File file = new File("config.yaml");
private Map<String, Object> config = new HashMap<String, Object>();

private DumperOptions yamlOptions = new DumperOptions();
private Yaml yaml;

private void setDefaults()
{
    config.clear();

    config.put(SERVER_NAME,     "Server");
    config.put(MOTD,            "Welcome to the server!");
    config.put(SERVER_PORT,     21020);
}

public void generate()
{
    setDefaults();
    save();
}

public void load()
{
        //To-do
}

public void save()
{
    //To-do 
}

public Object getObject(String key)
{
    return config.get(key);
}

public String getString(String key)
{
    return (String) getObject(key);
}

public int getInt(String key)
{
    return (int) getObject(key);
}

public double getDouble(String key)
{
    return (double) getObject(key);
}
}

Example of usage:

Config.getInt(Config.SERVER_PORT);
\$\endgroup\$

2 Answers 2

6
\$\begingroup\$

So far it's looking quite good, but I have a few comments:

Which file to load?

private static final File file = new File("config.yaml");

In my opinion it would be better to pass the File to load in to the constructor of this Config class, for flexibility. Now perhaps you only need to load a specific file, but in the future you might want to support multiple files. Who says that you always want to load config.yaml? Sometimes you might want to try different configurations, and instead of making backups of your config file and renaming files like crazy, you could instead load config-withsomespecials.yaml

private final File file;
public Config(File file) {
    this.file = file;
}

Speaking of flexibility, your setDefaults class works fine for your server, but you could make your Config class an abstract class, setDefaults as an abstract method and use a ServerConfig as a subclass in which you override this method:

// In your `Config` class:
protected abstract void setDefaults();

// In your `ServerConfig` class:
@Override
protected void setDefaults() {
    config.clear();

    config.put(SERVER_NAME,     "Server");
    config.put(MOTD,            "Welcome to the server!");
    config.put(SERVER_PORT,     21020);
}

These are only suggestions, if you don't want to make it that flexible, that's up to you.

<personal-opinion> As for the choice of using YAML.... it isn't exactly my favorite format. Let me guess, you're a Minecraft / Bukkit player? The Minecraft / Bukkit world is about the only place I've come across this format. If you have the ability to choose, I would personally recommend using Jackson, which is primarily used for JSON, although it has support for XML and even support for YAML. Jackson is wildly used, very flexible, and I think it is more well-documented than Snakeyaml.</personal-opinion>

\$\endgroup\$
5
  • \$\begingroup\$ {personal_opinion: " YAML is pretty common, just not in the Java world. E.g. Ruby is a YAML stronghold. It's better than JSON as a human-readable config format, and it has better support than INI files for complex data. Only XML offers more features, but they have little overlap in their uses, as XML is primarily an Extensible Markup Language. YAML is to JSON what Markdown is to HTML, except that YAML actually packs more features (such as a pluggable type system). "} \$\endgroup\$
    – amon
    May 10, 2014 at 18:11
  • \$\begingroup\$ @amon I'm glad there are others out there that like it, most of my bad experience with it is that I wanted to use tabs for indentation when manually editing but it did not allow that and didn't show much of an error message. Perhaps that has been fixed in various YAML libraries though. I'd still recommend Jackson though, makes serializing to/from POJOs so much easier. \$\endgroup\$ May 10, 2014 at 18:25
  • \$\begingroup\$ I wanted to use tabs” – there, that's your problem. YAML (like Python) uses significant whitespace, you can't mix tabs and spaces freely. \$\endgroup\$
    – amon
    May 10, 2014 at 18:27
  • \$\begingroup\$ I was originally using the Java properties class, but didn't like how it worked and it would be hard to use for more complex files. I was going to use JSON but ended up going with YAML. I'll check out Jackson and see if I can use it. \$\endgroup\$ May 10, 2014 at 21:13
  • \$\begingroup\$ I now changed the config class to an abstract class. The sub-classes handle the value names and defaults. \$\endgroup\$ May 10, 2014 at 21:24
6
\$\begingroup\$

I think this is really not so good:

Example of usage:

Config.getInt(Config.SERVER_PORT);

That's too much implementation to expose as an API. This would be a lot more natural:

serverConfig.getServerPort()

In your current code, I really don't see the point of the Map<String, Object> config field. Why not use native fields for the configuration? If there is other code that needs it this way, that should be part of the review.

Even if there is really a good reason for this approach, you should expose user-friendly API methods like:

public int getServerPort() {
    return getInt(SERVER_PORT);
}

and make getInt and the others private. No need to expose such implementation details.

Finally, as @Simon Andre Forsberg said, config.yaml should not be hardcoded. Actually I would make it a constructor parameter.

private final File configFile;

public ServerConfig(String configFilePath) {
    configFile = new File(configFilePath);
}
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Good point about the serverConfig.getServerPort() suggestion, +1 \$\endgroup\$ May 10, 2014 at 17:57
  • \$\begingroup\$ The reason for the map is so I can store all the values and access them by the key. The values are all different date types which is why there are objects. I could do the getters, but I wanted to keep this class more open if I added future stuff and not have hundreds of getters in it. \$\endgroup\$ May 10, 2014 at 21:15
  • \$\begingroup\$ @RyanPrintup I still don't link you should do it. If users of this class have to call the getInt and similar methods with the right constant name parameters, that makes a very awkward API. \$\endgroup\$
    – janos
    May 10, 2014 at 21:28
  • \$\begingroup\$ I took this method from the ListResourceBundle class that is part of the default packages. I would like to have just a get method, but a method can only return one variable type so it would have to always be an object which would require casting everyone a configuration option is retrieved. \$\endgroup\$ May 11, 2014 at 0:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.