[cbanek]

My #1 rule for existing codebases: Just because you wouldn't have done it the way they did doesn't mean they did it wrong.

I think it's developer nature to look at a huge pile of code that someone else wrote and immediately think: "This is a pile of crap. I can do better, so the first thing to do is rewrite all of this, my way (which just so happens to be _The Right Way_)."

Figure out what you're trying to do, and what is keeping you from doing it. Take an iterative approach to get things done. Realize that after 3 years, they have hopefully fixed a lot of bugs and got to a solution that is somewhat mature and better than you can do in a week.

[Jach]

There's a term for this, Chesterton's Fence: https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence

> let us say, for the sake of simplicity, a fence or gate erected across a road. The more modern type of reformer goes gaily up to it and says, "I don't see the use of this; let us clear it away." To which the more intelligent type of reformer will do well to answer: "If you don't see the use of it, I certainly won't let you clear it away. Go away and think. Then, when you can come back and tell me that you do see the use of it, I may allow you to destroy it."

That said, if you know how to refactor safely even without tests, you can make improvements. Since the questioner is working in JavaScript, Martin Fowler came out with a second edition of Refactoring with JS code that might be useful.

I've never read the first edition (in Java) but Michael Feathers' Working Effectively With Legacy Code is a frequent recommendation of mine. Its main goal is to get your mess under test, ideally tests that help you understand the system as well as improve its quality. It's organized as a set of questions one finds oneself asking in these kinds of code bases and some ways to resolve them: https://www.oreilly.com/library/view/working-effectively-wit...

[beachy]

The counterpoint to Chesterton's Fence is the psychology experiment with the monkeys, the stairs and the banana.

This is long but very pertinent to the "that's just how we do things around here" attitude:

> This human behavior of not challenging assumptions reminds me of an experiment psychologists performed years ago. They started with a cage containing five monkeys. Inside the cage, they hung a banana on a string with a set of stairs placed under it. Before long, a monkey went to the stairs and started to climb towards the banana. As soon as he started up the stairs, the psychologists sprayed all of the other monkeys with ice cold water.

> After a while, another monkey made an attempt to obtain the banana. As soon as his foot touched the stairs, all of the other monkeys were sprayed with ice cold water. It's wasn't long before all of the other monkeys would physically prevent any monkey from climbing the stairs.

> Now, the psychologists shut off the cold water, removed one monkey from the cage and replaced it with a new one. The new monkey saw the banana and started to climb the stairs. To his surprise and horror, all of the other monkeys attacked him. After another attempt and attack, he discovered that if he tried to climb the stairs, he would be assaulted.

> Next they removed another of the original five monkeys and replaced it with a new one. The newcomer went to the stairs and was attacked. The previous newcomer took part in the punishment with enthusiasm! Likewise, they replaced a third original monkey with a new one, then a fourth, then the fifth. Every time the newest monkey tried to climb the stairs, he was attacked. The monkeys had no idea why they were not permitted to climb the stairs or why they were beating any monkey that tried.

> After replacing all the original monkeys, none of the remaining monkeys had ever been sprayed with cold water. Nevertheless, no monkey ever again approached the stairs to try for the banana. Why not? Because as far as they know that's the way it's always been around here.

[Jtsummers]

I don't see how that's a counterpoint. Chesterton doesn't say "don't change it, that's how it is and shall be." He's saying, "Question why it's present before you attempt to change it."

Looking around, there were no obvious livestock or other reasons for the fence. But perhaps they're only there six months out of the year. Ok, make the fence a gate, and not a fence, so it can be opened more easily. Maybe it's no longer used for livestock, ok remove it altogether. But if it is used for livestock (but they were just over the hill that day), and the fence is removed, an error has been made (it existed for a reason) and the livestock can now wander beyond the lands they're supposed to be on.

[IshKebab]

Usually there is nobody to ask though, and if you're refactoring a mess then the code won't give you a clear answer. The only way to find out if the was a reason the fence is there is to remove it and see what goes wrong which is what you were going to do anyway.

[dotancohen]

Actually, the right place to ask is the unit tests or integration tests. Though there seems to be an inverse relationship between the code that needs refactoring and the prevalence of proper tests.

[shabbyrobe]

... which is a form of questioning why it's present. "Attempting to change it" in this situation is more like "submitting the pull request[1]" than "editing the code". The PR tends to only come together after one or more rounds of "git stash, try again".

[1]: or merging your branch, if you're solo.

[darepublic]

Short of doing that, talking to the stakeholders could help? I feel like if you understand the actual requirements you can safely make those refactoring decisions

[ahoka]

> The counterpoint to Chesterton's Fence is the psychology experiment with the monkeys, the stairs and the banana.

Except that experiment never happened. It appeared in some book without any source (a.k.a. pulled out of the author's ass). Interestingly there was a similar experiment, but with an opposite result; the old monkey re-learned the forbidden behavior from the new one.

http://www.throwcase.com/2014/12/21/that-five-monkeys-and-a-...

[distant_hat]

A lot of these parables are purely made up.

[cortesoft]

Except for most cases, this is the proper behavior. We share experience with each other, and we can learn from it even if we haven't personally experienced it, or even if the person telling us about it didn't experience it personally.

The anecdote says they don't approach the stair because that's the way it has always been, but the monkey might say, "we don't approach the stair because the last monkey in here told us that we would be sprayed with water if we did"

Learning from people who are no longer here is really important. Just because something hasn't happened in your lifetime doesn't mean it won't happen. There are so many stories from Japan after the Tsunami, about places that learned from their ancestors to stay safe from something they themselves had never experienced.

https://www.forbes.com/sites/davidbressan/2018/03/11/how-cen...

http://www.nbcnews.com/id/43018489/ns/world_news-asia_pacifi...

[peelle]

I head a tale that had the same lesson. Not sure where I heard it, but it went something like this:

>There was a Buddhist monastery up in the mountains. Every day at noon the head monk would call everyone together in the main courtyard to meditate.

>One day a cat that lived on the monastery grounds started to come to the courtyard during these meditation sessions. It would screech at, and scratch the monks while they tried to concentrate.

> After a few weeks the head monk got tired of this. He ordered that every day before they meditate, the cat be caught. They tied up the cat away from the main court yard. After meditation the cat was to be released.

> This went on for a few years and everything was fine. Then the head monk passed away. A new monk was appointed.

> A few more after that the cat passed away. The monks retrieved a new cat from a nearby village and they started typing up this new cat; as is tradition.

[sonnyblarney]

The monkeys are passing intelligent learned behaviour - the monks are not.

In the monkeys case - it makes sense they 'follow the herd'. The learned behaviour of their ancestors is saying something: "you're gonna get sprayed with cold water if you do this"

I'd imagine a lot of things get passed on this way, maybe some right some wrong, but I don't think this really helps the 'go your own way' case so much. That we generally follow crowds millions of years into our evolution might indicate that there is indeed wisdom in crowds. Though perhaps it's helpful to realize that they are not always right.

[neumann]

That was such an awesome story that I immediately tried to find the source. Unfortunately, all I found was an article and a stack exchange answer arguing that it is made-up by the authors of the book it first appeared in. [0] [1]

Nevertheless, I can come up with a few examples similar behavior in large organizations surrounding processes, workflows and general wisdoms.

[0] http://www.throwcase.com/2014/12/21/that-five-monkeys-and-a-...

[1] https://skeptics.stackexchange.com/a/6859

[e1g]

It’s an apocryphal story that became popular with “new age” and personal development crowds to encourage you to not be like the masses. It is endearing and illustrative of a useful truism, but sadly made up to be that way.

https://en.m.wikipedia.org/wiki/Hundredth_monkey_effect

[stuntkite]

I think it's always important to lead with an attitude of service and compassion for the labor of others before you and yourself now. Get upside down on that and it makes it really hard to be impactful.

It's easy to get mad at code when you first show up and just apply Chesterton's Fence and move forward. Personally, with legacy systems (and what isn't really?) when I show up to a new project I do my best to grok the internal knowledge and test it. With official tests but also building out an environment around it so I can test conclusions. I document what people tell me, what docs and scripts they link me to, and what my results are. Almost without fail, the institutional knowledge is cargo cult.

People rarely break out of their mold for what works for the thing they need to work on. This isn't a problem that they need to fix, but legacy systems always need someone to take the time to make more clarity. So as a new person I focus on building up dev and deploy environment from zero, over and over. It's kind of an ops process. For me it's about finding multiple/duplicate configuration points, processes, improving VM or container environments, profiling, making sure that if external services are being leveraged that they can be bootstrapped to local dev or at least faked, and confirm tests harnesses if they exist.

With a "big existing code base" I have found that almost universally, the full stack can't be bootstrapped on the devs computers. They rely on tricks to run parts and cloud services and undocumented scripts. It's never because it's impossible and as a new person, I am uniquely qualified to solve that. Once I have a smooth rebuild process I move inward and refactor for clarity and tests, being careful to not disrupt other people's working strategy.

So in my experience, the first 4 weeks are asking questions and documenting the lies while repeated bootstrap. Second 4 weeks is environment cleanup. After that it's learning the system from the outside in by making the entire environment single command bootstrappable. I keep a daily journal of my experience and try to build out docs/readmes to consolidate and correct the practices that were relayed to me in the first 4 weeks as well as new clean up that I'm working on.

Being a new person on a large legacy code base is powerful. You don't have the entrenched survival problems other people have. Clean up and document to explore and try to be an exponential productivity add while you do it. One of my favorite tricks is "adding" features by removing things that people did not remember they have put into production. At my current job, my first three months was -7000 and +500 lines to our master repo. My negative -7000 was more impactful than some people that pushed 100k+/- in commit. Also I find it's useful to wildly speculate about how things work and be willing to be VERY wrong early and often. People are more likely to tell you about how they think something works when they are correcting you than when you ask directly for help. No clue why that's a thing, but it is.

After you get done cleaning things up you can make your own horrible lore and mistakes and start the process all over!

[jrochkind1]

> People are more likely to tell you about how they think something works when they are correcting you than when you ask directly for help. No clue why that's a thing, but it is.

While some of that might just be people's enjoyment at telling other people they're wrong, I think it's actually often something else. It's just easier to _respond_ to something than it is to start with a blank slate.

The question "How does this work?" is a blank slate. Even when asking it for yourself about a gigantic new codebaes, you form hypotheses and then 'test' them in some way, as you say, "testing conclusions". When you give someone a theory, you've just given _them_ a 'conclusion' they can 'test' against their own domain knowledge. :)

[jamesmus]

If someone ask me to review a commit that contained +/- 100k loc, I think I'd reject it on principle.

[stuntkite]

Not one commit, total sum over 3 months. You would be right to refuse that in most circumstances.

[blub]

I've managed to finish reading the chapter with the example refactoring from Fowler's book before putting it down as parody.

He's part of a design school that likes to pulverise code into tiny functions with the goal of having code that consists only of assignments and function calls, reading like instructions in English to the computer.

Whoever was indoctrinated by Robert Martin, Martin Fowler & co should read Ousterhout's design book for a fresh perspective on this.

The value of a book like "Refactoring" lies in naming the refactoring (e.g. extract function), but please don't listen to the design advice.

[Jach]

Thanks for the pointer to Ousterhout, I don't think I've heard of him before but a lot of the Tcl people have put out good stuff.

Uncle Bob and friends definitely form their own "school". I think they call the "functions and assignment" style "Newspaper style", since for them it's kind of like reading a newspaper. Following it you can actually get a long way towards the dream of "self documenting code". I'm happy when people who believe in some specific school of thought get it to work for them, and even if I disagree on things at least there's consistency.

It's still never as clear as a literate program, though. And while Uncle Bob has been upgrading by learning Clojure, a lot of that school's work has assumed OO-in-the-style-of-Java5/C++98 environments. And to be fair that's a lot of the mass of legacy code we unhappily get saddled with. I think their work is less applicable and there are other schools to listen to when you have different languages or programming methods (FP, declarative, OO with a MOP, etc.).

I read the Clean Code book finally not too long ago, and while I've never argued with a book so much, there is value in it, and some programmers would do much better than their current flailing to follow its approach exactly. But these guys aren't my personal programming heroes, I'd rather be like a Norvig than a Jeffries (even if the latter is a perfectly acceptable engineer). But I can usually find value in all their works; philosophically I'm aligned with Bruce Lee, I'll take what I think is valuable and discard the rest.

[teknopaul]

Re"pulverise code into tiny functions with the goal of having code that consists only of assignments and function calls"

You dont refactor so its "so small it becomes functional". You remove/refacor code with side effects where possible. So that part of the code becomes functional. It does not matter if a function is a lot of code or a little as long as it does not impact the world outside it.

A small function that adds two numbers and as a side affect assigns some global state is to be avoided however small the function.

"Fuctional programming" is not just about writing lots of functions

[deanalevitt]

I apply Chesterton's Fence to pretty much everything, from code to design to business goals to politics. Back when I worked for a large tech company, I'd get deeply frustrated with their slow decision-making framework. I'd joined the company through their acquisition of our company and it felt like banging my head against a wall all day.

It was only a year after I quit that it began to make sense. Now I try to assume that people had good reasons for their implementations and I try to figure that out first.

[jimmy1]

Ok, but seriously I am genuinely curious, what is the reason for the fence in the picture? I see fences like this when I am out in the mountains camping or hiking. Is it to prevent some type of animal from roaming down the roads?

[Jach]

Every fence might be different! Clicking on the picture gives a description of that particular one's history. When I see fences/gates on forest service roads or access roads I presume one reason for them is to limit access. Roads require maintenance, having fewer vehicles driving over one means less maintenance. During winter season a fence could also be there to limit access for the purpose of reducing the need for search & rescue or towing services...

[jmalicki]

One common thing is for cattle to be allowed to graze on public land, and it keeps them from wandering out of bounds...

[scarface74]

With statically typed languages, you can do guaranteed safe, automated refactors. I’ll do those mercilessly without tests. I would be very wary of refactoring dynamically typed language.

[Jach]

I've broken code in Java because I inlined a private method as part of a refactor. I found out later that some other code was accessing it through reflection. Fortunately that other code was an old test, instead of production code, so when it started failing on the test automation servers I found out about it and could update the test to not do that. Then I did what I do frequently anyway, regardless of dynamic or static typing, which is to use ag[0] and increase my confidence there weren't any other surprise references. Of course there's always the possibility of some sadist splitting the method name into two strings and concatenating them later, ag's no guarantee either.

If you've ever done a partial build system you can also break downstream dependencies you didn't know about from even compiling, and not know about it until you try to integrate. (I've done this too, fortunately the integration happens as a gate to checking into the main code branch.) If you at least keep the source of everything locally, even if you don't build it, ag can help again.

My only point here is that static typing isn't enough if you're prone to fear-driven development; you have no guarantees, just things that increase confidence. Static type proofs are but one way to increase confidence. I'm happy Fowler decided to use JS for his second edition, since this particular line of FUD when it comes to there being some impedance mismatch between dynamic languages and refactoring is unmerited. The first auto refactor tools were made for Smalltalk, a dynamic language, after all.

To me the two keys to safely (at high confidence) doing any refactor are to first know what you're doing (and what a tool is doing if you're using one, I'm all for pushing for better tools) and second to do it in small bits with a tight feedback loop. As part of the loop after you make a change you use whatever methods (compiling for static type proofs, running tests, manually testing (REPLs help this a lot), sometimes just pure reason) to become sufficiently confident that you didn't break anything. Sometimes you still break things, as part of a refactor or just as part of regular development -- every bug filed is one that got past all of your reason, your compiler, and your tests, but don't let that fear drive you.

[0] https://github.com/ggreer/the_silver_searcher There are other tools too. Regardless of language, writing "grepable" code promotes a lot of nice qualities, not just easing refactoring.

[scarface74]

Even if someone were crazy enough to use reflection to get around accessibility for testing (which you should never do), once I saw that once, I would be judicious about using “Find all references for a class” - something else you can’t do reliably with dynamic languages.

As far as partial build systems, with at least C# that would still be caught when you do an integration build. But that still begs the question, why wouldn’t you use a proper internal package repo and proper versioning?

[emilsedgh]

This is quite true.

But it must be noted that the code may actually be bad. Or it may be bad due to a thousand valid reasons (time pressure, business changes, etc)

My personal approach is this:

* Write new code must be "good" (Whatever that definition is)

* As you iterate through old code, clean them up.

For example our own codebase is several years old Nodejs project. Mostly written in callback style with `async.auto` and it's really ugly and hard to maintain comparing to async/await.

All our new code uses promises and async/await. All old codebase uses callbacks. We have to use promisify a lot.

But without any hiccups, we are slowly and slowly moving towards a better codebase.

What helps "immensely" is tests. We have 2 different layers of test and I don't think anything would've been possible without them (although none of them are at 100% coverage. Nowhere near)

[bradleyjg]

I go back and forth if the code I'm adding should be what I consider good or if it's more important that it match the flow and feel of what's already there. A code base written in twelve different ways is usually even harder to read and understand than one that's "not good".

[vonseel]

Well definitely don’t write bad code intentionally!

[teddyh]

Certainly, but the definition of “good” changes over time. At one point, callbacks were good, and now they’re not. What happens when the definition of “good” changes a second time before the code base has been fully converted from callbacks? Well, you shouldn’t write bad code, so now the code has three code styles in it, and, more importantly, two styles of conversions: From callbacks to current-good style and from previous-good style to current good style. This is harder to keep in your head, so conversion now goes more slowly.

[tcgv]

I share the same perspective. The value of a code base following one single coding style/convention/architecture is huge, even if there's a better coding approach right around the corner.

[arvinsim]

Yes. Trying to refactor everything at one go is a disaster waiting to happen. It is always best to do it incrementally.

[Illniyar]

I don't think that's the right mindset to approach it. My experience usually is that it is a pile of crap, and I can do better, but I probably would make the dame pile of crap if I was under the same conditions of the original coders (time constraints and scope creep usually)

So change it if you can, but respect those who were there before

[rjf72]

I'd also add here that hindsight is 20/20. You don't really know what you're creating until it's created. Do it again a second time when you know exactly what you're getting at the end, alongside the challenges you'll face in the process of such, and you'd be able to do it faster, cleaner, and just overall better.

[aargh_aargh]

Intuitively, I often feel the same.

Joel Spolsky argues against that: https://www.joelonsoftware.com/2000/04/06/things-you-should-...

The Mythical Man-Month as well: https://en.wikipedia.org/wiki/Second-system_effect

[Illniyar]

It's worth mentioning that what both of these are talking about are rewrites using a new project in a all or nothing stake. Iterative rewrites are not subject to these caveats.

[cbanek]

I agree with both you and the parent that sometimes rewriting the code is best, and you can usually make a superior solution by utilizing lessons learned from the first version, either from rewriting or iterative improvement.

But I think the mindset to approach a codebase with is to be open to it, rather than dismissive of it. Code is such a stylized thing, and everyone has their own style and loves their own style most of all. It's hard and takes time to understand someone else's code (a vastly underrated skill IMHO). The temptation to find some reason to start over when it isn't exactly necessary is usually quite high.

Once you understand what the current code is doing, then you really have to apply your good skills on what parts to save, and what to change, and how that relates to the schedule and cost.

Also in a corporate setting, there can be political / organizational friction to throwing out code or starting over (and it rarely makes you friends with whoever wrote the first version).

[gorbachev]

Exactly!

The first version of anything almost certainly ALWAYS sucks. Usually the only way that doesn't happen is if the team developing the solution has prior knowledge of some kind...having built a similar solution before.

I used to always hate consulting for this reason. The company I was working for was always doing greenfield development for clients. We never, ever got to do v2, because the projects always transitioned to clients' teams. I can't remember a single project I was working on that I was happy with.

[yoz-y]

I'd say that you can make a better design, sure. But the implementation will almost always be worse at least for some time. An old codebase carries tons of bug fixes and workarounds, they may not be elegant, but they make it more stable.

A new reimplementation, albeit cleaner, will almost always be buggier.

[alexis_fr]

I think the book « The Pragmatic Programmer » advises to do refactorings twice: First refactor to see what it’s hitting, then rollback. Once you know what you’re hitting, format it and clear it up, commit, then perform the big refactor, and you have much better chances of going smoothly.

[james_s_tayler]

My number #1 rule for existing codebases:

Get it into a state that will support current business goals and roadmap while protecting current business value and users.

That definitely can and often should include refactoring. But. There are many caveats and your rule is absolutely one we should heed. Though I would hate it for the capable, talented, motivated individuals that are part of every informal (or formal) clean up crew to heed it at the wrong times. The times when (a) that's not the way anyone should do it (b) it's actively adding negative value along some dimension and (c) it's preventing the business from reaching the next level.

I think it's a skill in its own right to identify solutions that do and don't need replacing. I think a good rule of thumb is to ask "will this enable some new capability that we haven't been able to do before and that would deliver real value along some dimension to more than just me?". If you don't have a solid yes to that you shouldn't do it. Often I find if you can get an order of magnitude better along one or multiple (preferably multiple) dimensions then it's usually worth the effort.

[rhizome31]

Dead code, inconsistent naming, CPOLD versioning, god objects, giant functions with christmas-tree like block nesting, SRP violations everywhere... A lot of code written by professional developers is like this and this is objectively bad.

I've been lucky enough to work on several occasions with code that wasn't like this and it was a joy to maintain. Cherry on the cake, that type of code usually comes with some automated tests.

[james_s_tayler]

Don't forget the customary "Utils" class which always contains lots of hidden gems like reimplementing 75% of the string manipulation functions provided by the standard library - some being unique implementations, others being a single call to the underlying standard library :)

[ehnto]

I think it's beneficial to work within the idioms of an existing codebase to maintain predictability and reduce complexity overall. If you can improve the code by removing complexity, that's a win. Otherwise idiomatic code will stand the test of time in a complex codebase even if it isn't how you would have written it. One bad way of doing something is better than two completely different ways of doing the same thing.

[userbinator]

"When you see something that's old and it's been there for a long time and it's working, don't laugh at it."

(From a not-really-related yet quite interesting talk I watched a while back: https://www.youtube.com/watch?v=TQB0KK2rxcw#t=31m0s --- also featured on HN at https://news.ycombinator.com/item?id=18430512)

[ken]

It's developer nature to look at any code and think that -- especially your own! Or if you prefer the abstraction, perhaps to consider that "myself 3 months ago" is a different person than "myself today".

Every time I've gazed upon a new legacy codebase and said "This looks like junk", the system architect responded "Yes -- that's why you're here!"

[ummonk]

I mean it's pretty normal for a long existing codebase to be have had a lot of poor (in hindsight) design / architecture decisions in it and feel that if you got to write it with the benefit of hindsight you'd do a better job. It's only hubris to think you can do it in a week, rather than requiring as much time as the original (but with a better resulting codebase).

[cynicalreason]

I'm doing exactly this now ... the problem is multiple iterations of trying to set an architecture and a pattern to where things go. The architecture presented, if it was to be implemented that way would have been perfectly fine.

Now there are 3 iterations .. with 3 different patterns of what would be a state container, UI business flows split across 3 layers (data access/api, business/decision , view models) and view models (NOT VIEWS) sharing state.

What also doesn't help is the tooling is close to non existent .. I got so used to webpack / lib(framework) dev tools / hot reloading / all in 1 dependency management / intellisense

[rdiddly]

If your predecessors were indeed total hacks at programming, you can get all of the above. But it's a dubious pleasure. My head sustained a lot of scratching and smacking and my face was over-palmed.

[james_s_tayler]

It's not that they were total hacks. It's that they fell into the 1 standard deviation of the normal distribution that made them an average developer. Average devs produce the status quo, which is "software that works but that everyone complains about having to maintain". In my experience people with solid framework design and system architecture skillsets invariably fall into the top tail end of the distribution. They aren't like other developers. And they're rare. They can and often do get dubious pleasure out of making old code new again.

[speedplane]

Sometimes a very dubious pleasure. I work at a company where another product team decided to rewrite the entire product from Angular 1 to Angular 2 because it was so much better. I was shocked and dismayed, but didn't say much because I didn't feel like starting a fight I would lose, and it would suck me into a project that wasn't my primary focus.

[amoitnga]

This.

I see myself in your words.

[crimsonalucard]

Sometimes the initial design truly was a mistake.

[cc81]

Yes, and sometimes the people are fully aware of that as well. But even a shitty designed piece of code that has been running in production for year will have ironed out most of its bugs. There might be other designs that would be more maintainable but refactoring to reach that might be an investment that the company is not willing to make.

[debt]

"This is a pile of crap. I can do better,"

I'll take this one step further and say: if you think this, you're unqualified for the position. You are an amateur.

[badfrog]

Very many developers have this as their first thought whenever they see somebody else's code for the first time. Likely it's just a response to being confused about how or why something works. It's easier to think that somebody else is bad than to think that you've got something to learn.

What matters is how you handle that feeling. If you run with it and continue to operate as though the code is terrible, you're probably not going to get very far as a developer. If instead you take a step back, work to understand the code, and think about what led the original author to make certain decisions, you'll do quite well.

[james_s_tayler]

I'll take this one step further and say if you truly think trash fires are acceptable and there exists no developer who can replace a trash fire with something warm and inviting then _youre_ the amateur.

I get what you mean though. I've seen dreadful results of some people who tried to replace a WinForms app with a WPF app and the people who did the WPF version totally half-baked their underlying framework. It was very amateurish. In contrast the existing framework built on WinForms was 90% baked. It simply had a lifecycle the developer had to understand (very difficult, nobody got it right) and was essentially only missing a final inversion of control layer to tie it all together. I added the missing layer and time to implement a new screen and number of bugs plummeted.

I thought it was crazy in 10 years of that code existing and hundreds of people having worked on it that _I_ was the one to spot that and go "hmm, you missed a spot" and have such a dramatic impact with a single week of work.

It still fucks with me to this day that _nobody_ was either capable or willing. That just blows my mind. It stuck out like a sore thumb to me. It seems to be the pattern thats emerging in my career though. I've come to understand it's a rare thing. So, fundamentally I agree with you're advice as it applies to most people. I just know first hand there are some people out there that understand exactly how to breathe new life into old systems and how to prevent new systems from decaying.

[yakshaving_jgt]

I know this opinion is extremely controversial, but sometimes — usually as a result of inexperience — people do put out bad work.

Sometimes it's cheaper for a business to rewrite something (the size of that something is highly context sensitive), than it is to work around a poor approach to a problem.

I know we love to believe in the industry that everyone is a genius and any self-doubt is imposter syndrome, but as Camille Fournier put it: "This is Hallmark card pablum".[0]

[0]: https://twitter.com/skamille/status/1004735128726376448

[marcell]

I dunno, I've seen quite a few piles of crap in my day as a SWE. The bar is not that high to do better.

[theossuary]

I have to agree with the comment you're replying to with a caveat. In my (limited) experience balls of mud are created because the developers either didn't understand the problem or its domain well. So if you come into a "pile of crap" and have the same (or less) experience with the problem/domain and (almost definitely) less time, then I really don't think you can do better. Maybe you can do it badly, differently, and think it's better though.

If you have more experience in the problem domain (either business or technical), then you might actually be able to rewrite it much better. But then there are quite a few other problems: Do we have time to rewrite it? Will it integrate well? Are you actually solving the right problem? Is it so different people won't know how to use or maintain it? etc.

I think even if you're rewriting code, doing it in the framework that's already available, and making as much use of it as is reasonable, is the better option.

I don't think it really matters what the solution is, within reason, as long as everyone agrees to follow it.

[fsloth]

The point is "doing better" is pointless unless it serves a specific business value. Beauty is not a sufficient reason to refactor a code base in production - because refactor always has risks, and those risks should be offset always by some tangible expected reward.

[jamesmus]

Definitely. "Is it good enough for the business?". If yes, then leave it alone. Try to add new features in a safe way with automated tests.

I once worked a contract where it was expressly forbidden by the dev manager to refactor any code unless it was demonstrably necessary to support a new feature. Not much fun for devs at times but I respect the reasons behind the decision (esp. as it was a derivatives trading platfom and it can get pretty expensive pretty quickly when they go wrong).

[chillacy]

If you always think this you’re probably an amateur, but if you seen good code that wasn’t yours, then you might have a sense for things. Of course that requires that you read a lot of code.

[fsloth]

I have no idea who downvoted you. I agree completely.

In a business setting all decisions should come from a business analysis, not from an inner desire for beauty.

Beauty is important, but save it's search outside business hours.

[maxxxxx]

Sometimes it's true but you definitely should wait quite a while before you say it. First you need to understand what the current code does and also learn some history how the code has developed.