[Calavar]

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...

[vidarh]

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.

[shmolyneaux]

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.

[Calavar]

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...

[james_marks]

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.