Hacker News Reader

Comment by the-lazy-guy | Hacker News Reader

While obviously super-impressive, it is clearly not maintanable without AI agent. It has spinel_codegen.rb is 21k lines of code with up to 15 levels of nesting in some methods.

Compilers code was never pretty, but even by those standard, I feel like it is a very-very hard to maintain code by humans.

View on HN

Compiler code can be pretty if you have the time to maintain it. Compilers are some of the most modular applications you can build with hard boundaries between subsystems and clear handoffs at each level.

The problem is that people often do not have the time to refactor once they have gotten the thing to work. And the mess keeps growing.

And the migrations. Or rather all the half-started migrations that never get through meaning you have to deal with api v1,2,3 all the times.

Those are pervasive in any old and large project but in my experience especially so in compilers.

Management problem more than anything else, I feel.

Compilers should not have so much churn. You decide on a set of language features, stick to it and implement. After that, it should only be bugfixes for the foreseeable future till someone can make a solid case for that shiny new feature.

Scope creep is bane of most projects.

spinel_codegen.rb is an eldritch horror. I always get spaghetti code like this when using Claude, and I've been wondering if I'm doing something wrong. Now I see an application that looks genuinely interesting (not trivial slop) written by someone I consider to be a top notch programmer, and the code quality is still pretty garbage in some places.

For example infer_comparison_type() [1]. This is far from the worst offender - it's not that hard to read - but what's striking here that there is a better implementation that's so simple and obvious and Claude still fails to get there. Why not replace this with

    COMPARISON_TYPES = Set.new(["<", ">", "<=", ">=", "==", "!=", "!"])

    def infer_comparison_type(mname)
      if COMPARISON_TYPES.include?(mname)
          "bool"
      else 
        ""
      end
      # Or even better, strip the else case
      # (Which would return nil for anything not in the set)
    end

This would be shorter, faster, more readable, and more easily maintainable, but Claude always defaults to an if-return, if-return, if-return pattern. (Even if-else seems to be somewhat alien to Claude.) My own Claude codebases are full of that if-return crap, and now I know I'm not alone.

Other files have much better code quality though. For example, most of the lib directory, which seems to correspond to the ext directory in the mainline Ruby repo. The API is clearly inspired by MRI ruby, even though the implementation differs substantially. I would guess that Matz prompted Claude to mirror parts of the original API and this had a bit of a regularizing effect on the output.

[1] https://github.com/matz/spinel/blob/98d1179670e4d6486bbd1547...

The solution to this with Claude is to use a small agent harness and include refactoring steps once tests are written and pass. For some things you will need to include rules on coding style it should prefer. This is especially true for Ruby or other languages it has not seen as much training data for as for e.g. Python.

It's true that it's shorter, but I suspect that the if-return, if-return pattern compiles down to much faster code. Separately, this code was originally written in C then ported. There are reasonable explanations for why Matz has the code written this way besides the typical AI slop.

I'm skeptical of that reasoning because the original C wasn't too clean or performant either. For example emit.c from an earlier commit [1]

It writes a separate call to emit_raw for each line, even though there many successive calls to emit_raw before it runs into any branching or other dynamic logic. What if you change this

    emit_raw(ctx, "#include <stdio.h>\n");
    emit_raw(ctx, "#include <stdlib.h>\n");
    emit_raw(ctx, "#include <string.h>\n");
    emit_raw(ctx, "#include <math.h>\n");
    // And on for dozens more lines

to this

    emit_raw(ctx,
        "#include <stdio.h>\n"
        "#include <stdlib.h>\n"
        "#include <string.h>\n"
        "#include <math.h>\n"
        // And on for dozens more lines
    );

That would leave you with code that is just as readable, but only calls the emit function once, leading to a smaller and faster binary. Again, this is a trivial change to the code, but Claude struggles to get there.

[1] https://github.com/matz/spinel/blob/aba17d8266d72fae3555ec91...

I agree with the overall sentiment, but personally have grown to love if/return style.

I find it easier to reason about and as it ages, stays maintainable vs more elsif branches with multiple conditions each.

Obviously it doesn't matter much now if it's maintabable by hand or not. If code is passing tests and benchmarks, I am happy.

But I am not sure that huge files are easy for the AI to work with. I try to restrict the files to 300 lines. My thinking is that if it's easy for a human to understand the code, it will be easy for coding agents, too.

Reply on news.ycombinator.com