I tried to follow most advice given here, and this is my rewrite:
const std = @import("std");
const Allocator = std.mem.Allocator;
const Config = struct {
a: i32 = 1,
b: i32 = 2,
const MaxFileLen = 1024 * 1024;
const Error = error{ OutOfMemory, ConfigRead, ConfigParse };
pub fn init(
// What's the best name for the Allocator argument:
allocator: Allocator,
filename: []const u8,
// Is it idiomatic to accept the log scope this way:
log: anytype,
) Error!Config {
const str = std.fs.cwd().readFileAllocOptions(
allocator,
filename,
MaxFileLen,
null,
std.mem.Alignment.@"1",
0,
) catch |err| {
// I expect the caller to know that OutOfMemory isn't logged,
// and that ConfigRead and ConfigParse errors are logged.
// Is that a good idea?
if (err == error.OutOfMemory) return Error.OutOfMemory;
log.err(
"Could not read config file \"{s}\": {s}",
.{ filename, @errorName(err) },
);
return Error.ConfigRead;
};
defer allocator.free(str);
var diag = std.zon.parse.Diagnostics{};
defer diag.deinit(allocator);
return std.zon.parse.fromSlice(
Config,
allocator,
str,
&diag,
.{},
) catch |err| switch (err) {
error.OutOfMemory => return Error.OutOfMemory,
error.ParseZon => {
// Now this is annoying, I have to strip the trailing
// newline. Any easier way to do that, or a way to avoid
// having to do it at all?
var diag_str = std.ArrayList(u8).init(allocator);
defer diag_str.deinit();
const diag_writer = diag_str.writer();
try diag_writer.print("{any}", .{diag});
const diag_str_trimmed = std.mem.trim(u8, diag_str.items, "\n");
log.err("Could not parse config file \"{s}\":\n{s}", .{
filename, diag_str_trimmed,
});
return Error.ConfigParse;
},
};
}
pub fn deinit(
// Accepting a pointer here avoids being able to use "const"
// later. Is that alright?
self: *Config,
allocator: Allocator,
) void {
_ = allocator;
self.* = undefined;
}
};
pub fn main() !void {
// TODO: use default allocator when available
var debug_alloc = std.heap.DebugAllocator(.{}).init;
defer _ = debug_alloc.deinit();
const allocator = debug_alloc.allocator();
var stdout_buf = std.io.bufferedWriter(std.io.getStdErr().writer());
// Flush automatically? A good idea?
defer stdout_buf.flush() catch {};
const stdout = stdout_buf.writer();
// I feel like this "blocklabel: {}" syntax becomes
// unhandy if I set more than one value.
var config: Config = loadconf: {
const args: [][:0]u8 = try std.process.argsAlloc(allocator);
defer std.process.argsFree(allocator, args);
if (args.len != 2) {
std.log.err(
"Expecting config file name as single argument.",
.{},
);
return std.process.exit(1);
}
break :loadconf Config.init(allocator, args[1], std.log) catch |err| {
switch (err) {
// Using the knowledge that CondigRead and ConfigParse do
// not require any further debug output.
error.ConfigRead => return std.process.exit(1),
error.ConfigParse => return std.process.exit(1),
error.OutOfMemory => return error.OutOfMemory,
}
};
};
defer config.deinit(allocator);
try stdout.print("Configuration is {any}\n", .{config});
}
This brought up some more issues/questions:
- How do you usually name the allocator argument? It goes as first argument (after any
typearguments) usually? I have seen “gpa” in the standard lib, but that seems to be odd!? - Is it correct to use
log: anytypeto accept a logger (with a scope)? - Is it a good or bad idea to log only some errors and expect the caller to panic/escalate on other errors (e.g.
OutOfMemory) or do some logging at the caller site for certain errors if needed? - The logging function, e.g.
std.log.err, doesn’t seem to expect a newline (\n) at the end of the message. Now this got me in trouble as the string representation of the ZON parserDiagnosticscontains a trailing newline. How to deal with that in an easy and idiomatic way? - Should I always expect a pointer passed to
deinit? Or what should I do in case of myConfigstruct? (See also: When to use a struct vs a struct pointer)
Thanks for all your advice so far.