Remix.run Logo
eterm 8 hours ago

It's funny, because the wisdom that was often taught ( but essentially never practiced ) was "Refactor as you go".

The idea being that if you're working in an area, you should refactor and tidy it up and clean up "tech debt" while there.

In practice, it was seldom done, and here we have LLMs actually doing it, and we're realising the drawbacks.

hirako2000 8 hours ago | parent | next [-]

When the model write new code doing the same thing as existing logic that's not a refactor.

At times even when a function is right there doing exactly what's needed.

Worse, when it modifies a function that exists, supposedly maintaining its behavior, but breaks for other use cases. Good try I guess.

Worst. Changing state across classes not realising the side effect. Deadlock, or plain bugs.

aerhardt 8 hours ago | parent | prev | next [-]

When they decide to touch something as they go, they often don't improve it. Not what I would call "refactoring" but rather a yank of the slot machine's arm.

whimblepop 7 hours ago | parent | prev | next [-]

> In practice, it was seldom done, and here we have LLMs actually doing it, and we're realising the drawbacks.

I spent some time dealing with this today. The real issue for me, though, was that the refactors the agent did were bad. I only wanted it to stop making those changes so I could give it more explicit changes on what to fix and how.

localhoster 8 hours ago | parent | prev | next [-]

So I think theres some more nuance than that. A lot of the times, the abstraction is solid enough for you to work with that code area, ie tracking down some bug or extending a functionality. But sometimes you find yourself at a crossroad - which is either hacking around the existing implementation, or rethink it. With LLMs, how do you even rethink it? Does it even matter to rethink it? And on any who, those decisions are hidden away from you.

traderj0e 8 hours ago | parent [-]

It's only hidden if you don't read the code. Even if you don't, at some point you'll notice the LLM starting to struggle.

hyperpape 8 hours ago | parent | prev | next [-]

That's a real question, maybe the changes are useful, though I think I'd like to see some examples. I do not trust cognitive complexity metrics, but it is a little interesting that the changes seem to reliably increase cognitive complexity.

raincole 8 hours ago | parent | prev | next [-]

Really? I've never heard it's considered wise to put refactoring and new features (or bugfixes) in the same commit. Everyone I know from every place I've seen consider it bad. From harmful to a straight rejection in code review.

"Refactor-as-you-go" means to refactor right after you add features / fix bugs, not like what the agent does in this article.

xboxnolifes 5 hours ago | parent [-]

Notice how they didn't say to put it in the same commit. The real issue, and why refactor as you go isn't done as much, is the overhead of splitting changes that touch the same code into different commits without disrupting your workflow. It's not as easy as it should be to support this strategy.

Instead you to do it later, and then never do it.

raincole 5 hours ago | parent [-]

I think you're talking about a different topic unrelated to the linked article. In the linked article the LLM doesn't split it into several commits. If LLM had a button to split the bug fix and the overall refactoring, the author wouldn't complain and we wouldn't see this article.

bluefirebrand 8 hours ago | parent | prev | next [-]

There is a pretty substantial difference between "making changes" and "refactoring"

If LLMs are doing sensible and necessary refactors as they go then great

I have basically zero confidence that is actually the case though

ramesh31 8 hours ago | parent | prev [-]

>The idea being that if you're working in an area, you should refactor and tidy it up and clean up "tech debt" while there.

This is horrible practice, and very typical junior behavior that needs to be corrected against. Unless you wrote it, Chesterton's Fence applies; you need to think deeply for a long time about why that code exists as it does, and that's not part of your current task. Nothing worse than dealing with a 1000 line PR opened for a small UI fix because the code needed to be "cleaned up".

cassianoleal 8 hours ago | parent | next [-]

That is the flip side of what you're arguing against, and is also very typical junior behaviour that needs to be corrected against.

Tech debt needs to be dealt with when it makes sense. Many times it will be right there and then as you're approaching the code to do something else. Other times it should be tackled later with more thought. The latter case is frequently a symptom of the absence of the former.

In Extreme Programming, that's called the Boy Scouting Rule.

https://furqanramzan.github.io/clean-code-guidelines/princip...

traderj0e 8 hours ago | parent | next [-]

The Boy Scout "leave it better than you found it" is a good rule to follow. All code has its breaking points, so when you're adding a new feature and find that the existing code doesn't support it without hacks, it probably needs a refactor.

ramesh31 3 hours ago | parent | prev [-]

Indeed there's a distinction that needs to be made here between "not refactoring this code means I'll need to add hacks" and "Oh I'll just clean that up while I'm in here." The former can be necessary, but the latter is something you learn with experience to avoid.

esafak 7 hours ago | parent | prev [-]

Just do it in a follow up PR to keep them atomic. But do clean up; it's so easy now.