Hacker News new | ask | show | jobs
by yakshaving_jgt 1616 days ago
I have some shitty code that runs an important part of one of my SaaS businesses. I'd love to refactor it. I'd love to swap out the HTTP library that it uses with the better one that the rest of the project uses. I'd love to remove all the nesting and complexity. I'd love to make it not an eyesore.

But here's the problem:

- In the past five years, it has never failed.

- In the past five years, it has not needed to adapt.

- If I change it successfully, it will make exactly zero difference to my paying customers.

- If I change it unsuccessfully, it will break a core part of the system that people are paying for.

If I were to refactor it at this point, it's really only to pander to my own ego, which probably isn't a sensible business goal.

18 comments

We have very different definitions of shitty code. The shitty code I have dealt with in the past is unreliable, slow, hard to fix, and when someone does try to fix it, it doesn't stay fixed.

The code you are talking about here sounds very low drama, which is code that I love.

Maybe we should distinguish between "pretty" code and "shitty" code?

"Low drama code" is now officially the term I'm gonna use for reliable code.
This would be the crux of my disagreement with the phrasing of the article, even though I agree with many of the details, in specific contexts.

For me, shitty code implies poorly-named variables, confusing logic, brittle tests that couple tightly to the implementation and resist refactoring the code under test, etc. Shitty code makes it harder to understand what is going on, and makes it harder to adapt to new requirements.

At the other end of the spectrum is what I call "gold-plated" code. This is over-engineered, or just overly aesthetically pleasing for a practical and utilitarian tool that will eventually be replaced. It's code that was written to satisfy the perfectionist impulses of the author (we all have these), rather than to add business value.

In the middle is pragmatic, utilitarian code. The level of quality required to be in the sweet spot of course depends on the audience and longevity of the code in question. A language stdlib should be much higher quality in many dimensions than a startup's prototype. So shitty library code might look nicer than gold-plated startup code. It's all relative to your use-case.

In other words, as with about 50% of such thought-pieces that get posted here, I can summarize my objection as "don't give unidirectional advice when you're optimizing a U-shaped error function". You can err by making your code too polished and not polished enough. For a given startup, it might be that the most common error is to gold-plate, and so the advice "make it shittier" is right most of the time. But you're missing the underlying principle if you think that's the right advice all the time.

> "don't give unidirectional advice when you're optimizing a U-shaped error function"

I like it. Seems broadly applicable to any kind of advice-giving.

I think here it's more about the split between hypothetical and practical shittiness. In most business cases, code needs to change frequently and so writing code in a way that supports that is important. In GP's case, the code hasn't needed to change and so the fact it's overly complex (and presumably hard to change) doesn't actually matter in any practical sense.
> In most business cases, code needs to change frequently

That has not been my experience in several industries. Sure, there is always something that is being changed, but once the project matures the changes tend to be at the boundaries or around new features that were recently introduced. A relatively small % of the code is changed each year.

I think I took an online course on the applications of hypothetical and practical shittiness. That was presented out of the MIT online school, right?

Though, jokes aside, other than refactoring for efficiency, I think you're right. The term "shitty code" generally applies to the readability and not really the efficacy of code. We can usually go back and make it "prettier" with no real practical effect except to make ourselves look better during code review =/

I'm not sure you do. Code that runs fine for years but breaks if you change something is still shitty, albeit not necessarily shitty along the axis you currently care about.

One of the arts of software development is deciding when it is worthwhile to optimize along some quality axis and how much to invest.

That is, knowing when to churn the code out fast, knowing when to optimize and for and what and when to leave it alone are all much more subtle trade offs than they appear on the face of it.

I've been in the OP's situation before and the shitty code lasted until it's natural end of life successfully.

I've also been in the situation where everything was fine for years and then sudden new business priority changes dictated a slew of simple feature requests we couldnt handle.

Yeah, if someone's avoidance of what they consider shitty code leads them down the path of dancing with future hypotheticals it's a problem that needs to be fixed. Not by embracing shitty code, but by fixing their criteria for shittyness. Non-shitty can be full of elaborate dance moves with those hypotheticals, but most likely it's not. Non-shitty always stays clear of bug magnets though, and badly performed dance moves clearly are bug magnets.

As are "it works, so it must be good enough, right?" implementations, when it only works for circumstantial reasons. Code that would fail on encountering single-digit hours is still shitty code even when it works in 0-23 instead of am/pm and happens to never run before noon.

this is the crux of the matter - relative definition of “shitty.”

i’ve been told i wrote shitty code because i didn’t use abstract types or generics or i wrote overloaded methods that repeated themselves. but i knew the domain and i knew the task and my code fit within the domain and accomplished the task.

i’ve also supported code that was beautiful, idiomatic, flexible, and downright professional-looking, but it tried to do too much or it had severe antipatterns (especially in data access layer) and was a nightmare to troubleshoot. also, the lead developers on these projects seemed to be far less open to feedback than the people whom i’ve supported who wrote code like i did

Agree, "Shitty" is a very broad and relative term.
I'd take 'shitty code' than runs flawlessly for years on end, over non-shitty code that doesn't.

I've worked with too many 'architecture astronauts' over the years who over-design everything based on the latest-and-greatest thing they just read online someplace, and make it so complex no one understands how it works, or how to maintain it, and often doesn't do what it was intended.

I worked with both, 'architecture astronauts' (lol!) and shit-coders. I'd prefer there to be a balance. Code has quality can be measured in more than just one dimension. It's fine if code works, but it's even better if you know how. (No obvious errors vs. obviously no errors)
Reliable but bad old code becomes the piece labeled "do not touch" and can spawn bad code around it. By itself it never gets worse but it might be like the broken window theory of a code base.
In case you haven't heard of it before, that term comes from this blog post, quite a good read!

https://www.joelonsoftware.com/2001/04/21/dont-let-architect...

> Code has quality can be measured in more than just one dimension.

Well, if you don't need to change it (like the GP), most dimensions are moot. We used to call that "maturity", but the entire concept is out of fashion nowadays.

What leads to a correct architecture being probably the most relevant one by a long shot. Because a good architecture is a hard requirement for parts of your code to stop changing (the other being requirements maturity, that is not a dimension of code quality).

But yes, before reaching maturity, there are many dimensions of quality you may want to optimize for, and they conflict between themselves. Also, architecture astronauts get this name because they don't create good architectures, so you don't want them designing it anyway.

> Well, if you don't need to change it (like the GP), most dimensions are moot. We used to call that "maturity", but the entire concept is out of fashion nowadays.

Isn't that how you get ancient COBOL monoliths running on mainframes which everyone is afraid to touch?

Do you not need to touch it or are you afraid to touch it? Those things are different.
Pretty sure it's "afraid to touch it".

Haven't you heard horror stories about complete hackery and shitshow (sometimes not even code, but actual company processes) built around them because fixing it might as well have become impossible?

I agree but I'd go a step further and say that good design simply doesn't mean over using abstraction. It's really easy to fall into the trap of thinking "if I just pulled out this abstraction this would be so much cleaner!". This leads to the architecture astronaut situation you described.

In my humble opinion, what makes code non-shitty is that it is clear (well named variables, state manipulation is very explicit), well documented, as concise as reasonable, and handles the full range of inputs/outputs properly either by restricting the input/output space via types or assertions.

I think the problem is extensibility. Architecture astronauts tend to create abstractions, which are useful for eventual extensions to to the original design. Shitty code is hard to change, but astronautic code is difficult to understand and, more critically, often is planned for hypothethetical developments that not necessarily match with future needs.
It’s funny how the term “refactor” has evolved to include “rewrite/rearchitect.”

As originally championed and coined by Ralph Johnson and his protégés at UIUC, it was strictly a (semi provable) safe transformation to the code structure/syntax (usually motivated by trying to make the code easier to understand and/or maintain). Since it was a new word in those days, we still used the terms rewrite and rearchitect as the primary lexicon of “change the code” and “refactor” stated to its niche.

Of course pure refactorings sometimes aren’t as pure as we hope. And they’re often coincident with rewrites and rearchitectures, because we often transform the code first to make a change in behavior easier to implement.

I’m not complaining about the use here. A drop in replacement of an http library would indeed be a true refactoring if indeed none of the externally viewed behavior changed, but as the OP has undoubtedly learned through their own experience, most refactorings end up being something like refarcotring++.

I’m mostly just marveling how in 30 short years, I got to witness the transformation of this term in coumputing lingo.

You're still maintaining it. In the sense that _if_ it fails in the future (which is admittedly unlikely, given that it hasn't done so in the past 5 years. But as stock investment services say: Past results are no guarantee for the future!) - you must fix it, and fairly quickly. So you need to maintain the toolchain and be at least sufficiently familiar with it that you or somebody in the team can hop in and fix things if needed.

That, on its own, is a cost, and that cost can warrant a refactor. To go for an extreme:

It's written in COBOL, it is the last cobol code in your entire code base, and the only engineer with any experience in cobol is about to retire.

If that happens, I'd refactor it. If you don't, and 2 years hence the customer calls in a panic that the product they are still paying for isn't working, you're really screwed. I doubt the customer is willing to wait for you to __learn COBOL first__.

But, merely 'the code is not at all written with modern practices and our current team's preferred style and such in mind; it wouldn't pass code review right now?' - yeah, don't refactor that, absolutely.

> If I change it unsuccessfully, it will break a core part of the system that people are paying for.

This is such an underutilized heuristic. I find phrasing the question as “Should we risk having a detrimental impact on customers for an aesthetic desire?” helps me to either get my head straight or to realize that the refactor has genuine value beyond aesthetics.

Well, that's humblebragging for ya^^ - your code isn't shitty, au contraire mon ami, if it ain't broke don't fix it.
Complexity is a function of how hard something is to understand multiplied by how often it needs to be understood (and modified).

If it is impossible to understand, but never needs to be understood, it has no cost... Until you or someone else needs to understand it, that is :)

> If it is impossible to understand, but never needs to be understood, it has no cost

You've just argued that difficulty of understanding times need to understand is cost, not complexity. It doesn't make sense to claim that complexity becomes simplicity if you no longer need to understand it.

> Complexity is a function of how hard something is to understand multiplied by how often it needs to be understood (and modified).

I like this. Sums it up pretty nicely.

This is not a shitty code. It does the job. It is shitty when it becomes the source of issues.

What you are describing is a perfect code, made for the sake of perfectness.

I have one single-file single-function spaghetti monstrosity running one non-critical service since 2009. I've contemplated rewriting it, but... it still works. As long as the db is there and the file itself is there, it'll keep going. And there are users using it. It's become my favorite wart. It will die with the server it's on.
I feel you. Some people are saying that, if it works, it's not shitty code, but it's not the truth.

I made a service that was reliable, but only because there are a lot of ifs inside the code that fixed bugs. It was really shitty code that somehow worked reliably.

The service was replaced because someone else rewrote the whole thing in another language.

> If I were to refactor it at this point, it's really only to pander to my own ego, which probably isn't a sensible business goal.

But it could be fun! You are aware of the risks, so if you'd really love to do it, knock yourself out! :D

Another perspective: It has occupied your mind for such a long time, why not get rid of it?

To paraphrase John Ousterhout: If a system has a few parts that are very shitty, but those parts almost never need to be touched, then they don't have much impact on the overall shittiness of the system.
> - If I change it unsuccessfully, it will break a core part of the system that people are paying for.

Every refactoring comes at risk of breaking something. So if you refactor this code often, you are risking an outage--the very thing you are trying to prevent!

As others have said, if it ain't broke, don't fix it.

It sounds to me like said shitty code got lucky in the tech debt casino. But great! Look at it, so you know how it works, but don't mess with it.

> Every refactoring comes at risk of breaking something.

Refactoring is code transformations that don't affect functionality. If something broke, that's a failed refactoring.

Which you discover after deployment...same deal
Manual refactoring is certainly risky. So it should be set about with automated tests, specifically to protect the refactored functionality.

But I think any legitimate refacvtoring can be carried out by automatic tools, with guarantees of unchanged functionality/semantics.

The number of refactorings possible for any piece of code is almost infinite; you can't just let some tool loose on your code to perform all possible refactorings. That's madness. You need a human to judge which refactorings are beneficial. But the actual transformation should be performed by a tool that knows the language, and can guarantee semantic equivalence.

And you still need tests. You can scrap them once the refactoring has been shown to have not changed the functionality.

The problem is that you cannot evolve that part of the code.

All is fine as long as you don't need to.

When somebody change that library without approval, it will be a pandora box.

So it's a ticking time bomb.

From the OP:

> - In the past five years, it has never failed.

> - In the past five years, it has not needed to adapt.

It's not a ticking bomb. The longer this runs, the less likely it will be a ticking bomb. Nobody is trying to adapt the code. Nobody is changing a library.

Haha, I've worked at places like this where the whole business depends on a system that nobody touches and integration tests are put around it and the business never makes any progress and all ideas are stalled around "sorry Bob's old code prevents this from being possible".

The fact that in some instances this works is not a cogent argument for writing shitty code... there are some places where it is an absolute f-ing disaster!

This also has a tendency to create ever-building organizational pressure for the Big Rewrite. And when the pressure overcomes the resistance, the dam bursts and all the debris turns into a never-finished ball of mud because there are so many business hopes and dreams pinned on the new system. Identifying release valves along the way can prevent that quagmire.
Story time!

At my previous place they had an ancient java application that powered their entire business. It had some reasonable parts, but some of it was a total mess from top to bottom. Leaky abstractions, multiple layers of incomprehensible inheritance, helper classes everywhere etc etc, the usual things you expect from a decade old java application.

A fair chunk of the code quite literally hadn't changed for a decade. There were classes written by the first few employees at the company and hadn't changed since that time. Indeed, no time was invested in fixing it because of the 'if it ain't broke, don't fix it' paradigm, which was correct for the first 10 years or so.

One day, there was a huge shift in the direction of the company. They wanted their java application to become multi-tenanted SaaS platform instead of single-tenanted system. I was put to work on the task, and found we had to touch virtually every part of the tech stack. It was an utter nightmare, and frankly we still had to cut corners because it would have taken another 10 years to do it properly.

My point is anyway assumptions are just that, assumptions. Assuming code is never going to change is as dangerous as assuming how could might change and planning for a thousand possibilities. In the case of my previous company, assuming multi-tenancy would be coming along would probably slowed them down significantly and was well beyond the scope of the company at the time. However, if they had spent a bit of time polishing the turds, and putting in place proper testing, it would have made life a hell of a lot easier when strange requests come along.

What assumptions does it make? What if one of those assumptions fails? For example, what if it's never called with a certain combination of parameters because nothing generated them until a new process was added somewhere else and that combination of parameters now cause it to fail? What if the format of a parameter changes slightly and it fails on that?

We have code like this where I work and cases where a change to something has caused that one rock solid process written by an intern in 1998 that few people even remember exists to suddenly start to fail. I would call it a ticking time bomb, and if it's one that will explode in a big way, it might be worth analyzing it to see how it will react to non-obvious parameters or environments.

What happens when a vulnerability is discovered? What happens when the library's dependencies gets deprecated?

I mostly agree with you, but 5 years is not very long time either.

> What happens when a vulnerability is discovered? What happens when the library's dependencies gets deprecated?

The same as what happens with non-shitty code, you fix it.

Is it to pander to your ego, or is it to make it easier for a new joiner to onboard successfully into the tech part of the business?
It's a solo project, so definitely the former.

As with anything in engineering, the answer is always "it depends". I think programmers too often take for granted this idea that all code must adhere to some strict "clean code" rules, or follow some kind of laws or naming conventions or whatever.

"if it does not break, don't fix it. If you fix it it breaks, you must hold both ends"
Undefeated for 5 years.

That sounds like amazing code, shitty code breaks prod.

> - In the past five years, it has never failed.

Five years! Reminds me of the first five years of the Challenger's operation, in which it never failed!

poor analogy - physical things wear out over time, running code doesn't suffer from fatigue.
Code does suffer from a kind of fatigue over time, but related to the environment around it changing, called "bit rot". If you're able to control the environment (and you can replace the hardware components which will eventually fail), then sure, the software doesn't really age. However, if your operating system, compiler, standard library, host hardware, network layer, or other things change, then the software will "fatigue" and eventually fail.

This can be mitigated, look at languages which compile to byte code and are executed with a virtual machine. If it's sufficiently well-defined and stable, then the software can, more reliably, survive underlying hardware changes. Changes to network protocols are often out of our hands, though. Consider the push for HTTPS as well as updates to the HTTP spec itself. A web app from 2003 may or may not function, as written, today. Or it may "function" but users will see random failures (the browser insists on ignoring those HTTP requests for images hosted on another domain, for instance). And even if all the connectivity parts work, you rely on the client to still be able to render things correctly, good luck with your circa 1998 ISP hosted homepage's <blink> tags showing up correctly these days (which is probably a good thing, though).

The more control you have over the system, the less likely your software will be struck by "fatigue". The embedded systems I helped maintain that were written in the 70s and 80s and still running on the same hardware, no bit rot to speak of. Desktop application written in 1995 using Visual Fortran, massive bit rot.

Poor understanding of the facts! LOL

It didn't wear out over time. The design of the seal was bad from the beginning.

If you don't count security issues.
It's people who read it that do