Hacker News new | ask | show | jobs
by clumsysmurf 1224 days ago
> They take the extra step to make sure the next person won't have to spend the same level of energy fixing the same issue, or eliminate the problem class altogether for their team.

Conversely, I find it frustrating when engineers do the absolute minimum, avoid refactors, and put the next person at a disadvantage ... all while their velocity is recognized by management as good.

5 comments

I'm conflicted about this. Definitely am a fan of "leave it better than you found it", refactor as you go, etc.

But when it comes to bug fixes, the "absolute minimum" is often the best approach -- it can be explained to demonstrate that we know precisely the root cause of the problem, it can be reviewed for correctness readily, and we can feel good that we are not adding a lot of new behavior that can have downstream effects. The "defensive programming" approach.

A refactoring that makes the problem go away forever is an awesome tech debt project, but when you have a hot problem and someone puts up a 1k LOC PR to refactor the problem away it often introduces so much uncertainty that the improvements aren't worth the risk. You have to start asking questions like how long has this person been around, do they understand what they are doing here, have they really tested this fix or does it just have good "coverage", do they really understand the root cause of the problem or is this just a "refactor and pray it goes away" fix...

A small-as-possible fix relies much less on the credibility of the code author and much more on the fix being self-evidently correct.

"Absolute minimum" in practice usually means hard-coding a special case/workaround directly into a live prod instance. That is the quickest and 99% of the time won't cause any operational issues in the short-term. It happens all the time and contributes enormously to technical debt.
It's a judgement call. If you're maintaining software, it's true that every time you touch the code -- even to fix a bug -- you're taking a risk that you're introducing a new bug. Refactoring software is an even greater risk.

So, in general, the instinct is (and should be) to touch the code as little as possible. However, that's not always the right answer. The right answer is to do a risk/benefit calculation and choose the approach that is most likely to lead to an overall improvement.

Sometimes, that means change as little as possible to fix a bug. Sometimes, that means refactoring a major piece. It all depends.

> I find it frustrating when engineers do the absolute minimum, avoid refactors, and put the next person at a disadvantage

If such engineers don't care about other or future engineers who will have to touch their work, they should at least care about themselves. Perhaps they've never heard the maxim: work to make the life of the next engineer to touch the code easier, because the "next engineer" will probably be you.

There is some unfortunate tension between the two maxims of "cleanup as you go" and "keep PRs focused." An idealist will tell you, "why not submit the cleanup in a separate PR?" But in reality, there is always overhead to each new PR, and the code most amenable to "cleanup on the go" seems to be the code that sits just below the start of that overhead.

It's especially frustrating when "cleanup the code" means "change one line in a few dozen files," because this results in a PR with a high number of changed files. Even if each change is only one line, the high number in the "top line metric" (no pun intended) of "files changed" makes the PR look unfocused, and a lazy reviewer might waste everyone's time criticizing that. So the path of least resistance is to avoid even mentioning the cleanup. Repeat for every PR for the lifetime of the codebase, and your code quality will deteriorate from death by a thousand cuts.

> I find it frustrating when ... recognized by management as good.

... but you're not the one writing the checks, remember? Management is. They don't care about quality. They can't even define it, much less recognize it. They care about delivery dates. Delivery dates are easy to measure. So that's what matters.

> put the next person at a disadvantage

The first guy was at the same disadvantage - which was that he was under an unreasonable deadline that didn't leave enough time for quality improvements. So you expect him to sacrifice his reputation so that you don't have to sacrifice yours?

You're describing dysfunctional or bad management. Not all companies are like that.
Doing these kinds of unnoticed forward-looking remedial things provide the doer with good conscience and inner satisfaction, but do not put the doer on a fast track to career success and riches. It's the nature of the economic beast.

And thus the current state of "civilization".