search

Civcraft / Bastion

Do not open issues here; open them on the maintained fork @ DevotedMC
https://github.com/DevotedMC/Bastion
2 stars 9 forks source link

Odd usage of java.io.File #24

Closed psygate closed 7 years ago

psygate commented 9 years ago

ConfigManager uses String concatination instead of the File(File parent, String child) constructor

Maxopoly commented 9 years ago

Would changing this provide any advantage? It's obvious what it's doing and performance isn't an argument here either.

ProgrammerDan commented 9 years ago

Not really. The compiler will just convert it to a StringBuilder anyway, so efficiency isn't an argument.

It's more a semantic "correctness" -- so not a high priority. I could lower it.

psygate commented 9 years ago

It does some additional checks and path normalization, which can enhance the compatibility with different file systems than NTFS, FAT and Ext. It's not exactly the most urgend thing ever, but see here: http://www.docjar.com/html/api/java/io/File.java.html Line 289.

Maxopoly commented 9 years ago

This is a very weird approach anyway. Instead of hardcoding parsing from a file named "config.yml", and reimplementing a getter for every primitive, spigot's default config system should probably just be used in that class. The whole loading of the config and exception catching could be replaced with

Bastion plugin = Bastion.getPlugin();
plugin.saveDefaultConfig();
plugin.reloadConfig();
config = plugin.getConfig();
ProgrammerDan commented 9 years ago

Quite so; I see no reason not to correct it to use the Spigot loader system.