\$\begingroup\$

string value = text[assignIndex + 1 .. $].idup.strip();

Since you're using stripRight on the line above, why not stripLeft here? Also, instead of doing idup.strip , use strip.idup - that way you aren't allocating spaces that you then strip off and ignore, wasting memory.

configFile.close();

Unnecessary - File will auto-close when leaving scope.

void writeConfig(File configFile, const string[string] configData) { configFile.rewind(); string[] configKeys = configData.keys(); string textBuffer;

Consider using Appender!string instead of string here. As the name indicates, it's optimized for appending, while string is more a jack-of-all-trades.

Next up:

(configData[configKeys[varIndex]] || "null")

|| always returns a boolean result, so this will always evaluate to true , or 0x01 , so every single line you write will be corrupted. You can use this utility function:

auto or(T...)(T args) { foreach (e; args) if (e) return e; assert(0); }

Usage:

configData[configKeys[varIndex]].or("null")

All-in-all, writeConfig looks a bit curious. I assume it's written the way it is in order to keep comments, and perhaps as an attempt at optimization. However, it's using startsWithAny on every line, giving it a N^2 complexity. This can be improved by looking up the prefix in the AA:

const assignIndex = stripped.indexOf("="); const prefix = stripped[0..assignIndex == -1 : 0 : assignIndex]; auto var = prefix in configData; if (var) { textBuffer ~= prefix ~ " = " ~ ((*var).or("null")) ~ '

'; configKeys = configKeys.remove(varIndex); } else textBuffer ~= text ~ '

';

Another problem with using startsWithAny is that a variable called foo will overwrite one called foobar , if they happen to be arranged that way in the AA keys. Your solution also discards comments on the form var=value #comment .

Next:

File configFile = File(configPath, "w+");

You're trying to read lines from this file. Use "r+", not "w+".

The problems with writeConfig are generally also present in setVariable . setVariable is also somewhat confusing - when would you set the value of a setting, but not update the string[string] ?

Lastly, have you even tested this code? As pointed out, it will write corrupted data to file, it will discard comments, it will overwrite variables with similar names, and it will ignore file contents.

I would suggest in the future you write at least some kind of unit tests to check that the code does what you want it to. Something along the lines of:

unittest { import std.array : array; import std.algorithm.iteration : map; enum filename = "config.conf"; // Create initial config file { auto f = File(filename, "w"); f.writeln("#comment"); f.writeln("var2 = foo"); f.writeln("var1 = bar"); } // Read contents and check that correct data is read auto cfg = readConfig(filename); assert(cfg.length == 2); assert("var2" in cfg); assert(cfg["var2"] == "foo"); assert("var1" in cfg); assert(cfg["var1"] == "bar"); // Write back and check that correct data is written writeConfig(filename, cfg); auto contents = File(filename).byLine.map!idup.array; // Trailing \r included in byLine is bug #11465, and a windows-only problem. assert(contents[0] == "#comment\r"); assert(contents[1] == "var2 = foo\r"); assert(contents[2] == "var1 = bar\r"); }

Now, I can't in good conscience write only negative stuff, and there are certainly good things in your implementation. Your naming is great - names are explanatory, not too short and not too long. Comments are used when they are useful, not too many, and not too few. Functions are of good length, and the control flow is easy to follow, and utility functions are introduced where sensible. Imports are tidy, and selective imports are used to give quick information about which function comes from where. All in all, it's a pleasant read, marred by implementation problems.

If I were to write an implementation of the functionality you have, I would also encapsulate it in a struct or class:

struct Config { string[string] _variables; this(string filename) { // read into _variables } string opIndex(string name) { return _variables[name]; } void opIndexAssign(string value, string name) { _variables[name] = value; } void save() { // save to file } }

This way, I have all the functionality inside a nice little box that I can pass around, and I'm free to keep other information like comments in the data structure, without exposing that complexity to the user of the code.