Free after re-assignment?

[stray briar]

This smells like an anti-pattern, but I'm not able to see a better way at the moment. Am I missing something, or is this a normal way to "build" something based through a series of steps?

I'm talking about the times I do const old_path = path; defer allocator.free(old_path).

pub fn to_file_path(allocator: std.mem.Allocator, url_path: []const u8) ![]u8 {
    std.debug.assert(std.mem.startsWith(u8, url_path, "/"));

    // make it a ./ relative path
    var path = try std.mem.concat(allocator, u8, &[_][]const u8{ &".".*, url_path });

    // friendly urls
    if (std.mem.endsWith(u8, path, "/")) {
        const old_path = path;
        defer allocator.free(old_path);
        path = try std.fmt.allocPrint(allocator, "{s}{s}", .{ path, "index.html" });
    }

    // resolve the (relative) path to a file
    const old_path = path;
    defer allocator.free(old_path);
    path = try std.fs.path.resolve(allocator, &[_][]const u8{path});

    return path;
}

[upbeat scroll]

Code could just be tightened up.

Like, if you just look a the last 3 lines, why not do:

defer allocator.free(path);
return 
std.fs.path.resolve(allocator, &[_][]const u8{path});

And, if you're willing to repeat the logic for making it relative, you can clean up the top part AND remove an allocation.

    const path = if (std.mem.endsWith(u8, path, "/")) blk: {
        break :blk try std.fmt.allocPrint(allocator, ".{s}{s}", .{ path, "index.html" });
    } else {
        break :blk try std.mem.concat(allocator, u8, &[_][]const u8{ &".".*, url_path });
    }

(different ways to write this, including extracting to it's own function, but this should give you the ideal)

[lofty saddle]

arraylist version:

pub fn to_file_path(allocator: std.mem.Allocator, url_path: []const u8) ![]u8 {
    std.debug.assert(std.mem.startsWith(u8, url_path, "/"));

    // make it a ./ relative path
    var path = std.ArrayList(u8).init(allocator);
    defer path.deinit();

    try path.appendSlice(".");
    try path.appendSlice(url_path);

    // friendly urls
    if (std.mem.endsWith(u8, path.items, "/")) {
        try path.appendSlice("index.html");
    }

    // resolve the (relative) path to a file
    const resolved = try std.fs.path.resolve(allocator, &[_][]const u8{path.items});

    return resolved;
}

[upbeat scroll]

If you go that route, worth considering calling ensureTotalCapacity(url_path.len + 20) and using appendSliceAssumeCapacity

[lofty saddle]

I can't find a super clean way to do processing steps where the resources are cleaned just after the next step. You could skip that entirely and defer deinit() each step in the whole function scope, or even group all the processing together in an arena allocator and defer deinit that for the same effect.

[upbeat scroll]

(the +20 could be more elegant)

[stray briar]

ya I did consider doing an arena allocator for these things. that's probably the cleanest, with respect to source code readibility

I do like the arraylist appending though.. could possibly wrap it in a builder pattern api if I want this to be a thing

ya... that seems pretty good