Ask HN: Codebase at my work is a complete mess, what should I do?
After few days/weeks of getting to know the whole codebase I finally moved to a specific project I was suppose to work on and I found out the code is a complete mess. Often I find parts of code that unintentionally affects other parts of code. Some parts are just copied and left there doing nothing. Even names of classes/variables are sometimes useless and project structure is unintuitive and seemingly without rules. It's spaghetti and relatively big spaghetti (tens of thousands of LOC).
I started "repairing" the project but there're no tests to check whether my adjustments are correct. I'm so frustrated and depressed by it. The job basically turned into something I hate - I'm just rewriting the code! The person in charge of this project was working on it alone and from the outside it all looks fine and it's working. So this makes the higher people think that it's all just fine. I really don't know what to do. Should I just go and basically say that this person did a bad job? I don't feel very comfortably doing that because (a) I'm a relatively new hire, (b) the person in charge is working there for about two years and (c) I'm much younger than she/he is both professionally and biologically.
I've already made some comments about rewriting it and the response was basically "ok".
345 comments
[ 2.1 ms ] story [ 304 ms ] threadThat's not likely to get good results; you'll want to do more prep work to ground "did a bad job" in data. That data will also be useful in making the "rewrite everything" decision, should things come to that.
Start off at the organizational level: how did this code get into production? You're not going to get anywhere arguing for code quality at a place that doesn't value it. Is the rest of the codebase like this? If it is, you may be facing a larger company-culture issue.
Making real improvements is going to be seriously difficult without automated testing, so you'll probably need to add testing incrementally. Maybe start off by reproducing a production bug in a test and then fixing it - this delivers immediate business-visible value faster than stopping the world to write tests. Whenever you make changes, wrap the affected part in some more tests. Are there manual acceptance tests? Consider automating some of them.
Finally, be prepared to bail if nothing works - some places suffer from the "Market for Lemons" problem, where good developers who join figure out it's a dumpster fire and leave behind only bad ones.
Ok, more seriously: that’s the way existing code is about 90% of the time - it’s the reality of working in our field.
It may or may not reflect on the original developer. Maybe they were learning a new framework. Maybe they were rushed. Maybe it was originally intended as a prototype but ended up in production. Maybe the requirements have grown or changed significantly since it was implemented and the original design didn’t scale well (this is to blame for most of the awful code where I work). Maybe they’re just not very good.
If you’re going to take this to management, I would not bad-mouth the original developer. I would suggest framing the issue as the code not scaling well to the new requirements.
Then give them a few options with trade-offs. It could be that management is willing to spend the next few years playing but whack-a-mole to avoid an extra couple weeks on the project to put it under test and refactor a bit. It could be they think the fix up is worthwhile now if it’ll save them a month’s worth of development time in the next few years, but not if it’ll only save a couple days. It could be that they intend to rewrite the entire area of the product in 6 months and only want to invest the minimum effort in changes until then.
Well said. There are many possible causes behind a poor codebase, and it's rarely useful (and almost never persuasive) to go into finger-pointing mode. The issue for the OP is: from the business's perspective, is the poor code a problem? If it's not, can they live with continuing to work on this code in this job? And if it is: what's the next step, and what can they contribute to making it happen?
This. If there are deficiencies in the code base, focus on those, their effects, and the gains from fixing them. Blaming people is rarely a good idea, for many reasons.
If the original dev worked on the code base for a long time, they're probably already aware of them. God knows I'm all too aware of the deficiencies in code I wrote years ago, but that's frequently how things look after years of updates and feature creep. Everyone on our team has code like that, and when new guys come in and point them out, we just sigh and say "yeah it would be great to rewrite that knowing what we know today". But budgeting time for that is not easy.
Edit: Also, like others said, I think you need to adjust your expectations. Working with code like that is life, and you should appreciate the fact that you, with your outside perspective and youthful energy can make a change for the better. But it's going to involve lots of refactoring and test writing.
The "Legacy Code" book referenced in another post is a great overview of this. I'd say of all programming books I've read, it's one of the ones that are most relevant to my day to day work.
Adding on, the original developer is almost certainly aware of definciencies in the code. My guess is that they are largely a result of rushed deadline, changing scope, and feature creep than they are the result of bad engineering. When you need to get a new build out at 11pm because you have an angry client on the phone, it can be tempting to say “I know this is bad practice, but I’m just throwing this in the controller to get it working for now. I’ll fix it in the morning.” Especially at a small company without solid engineering practices, this happens enough times and you’ve eventually got an application with no meaningful organization or architecture.
Having been in charge of an app that started to look like this, i know that I would have loved a new dev to come in and say “I want to familiarize myself with this codebase by writing some tests, and seeing if there is any basic refactoring I can do. Where is a good place to get started?” The original developer probably wants to write those tests himself, but either has too much on this plate, or doesn’t realize how much time he’s wasting by working on a fragile codebase.
All in all, as other posters have mentioned, stay positive, and see how you can clean up and improve your little corner of the world.
One last point, I might take this as a small red flag. Don’t quit over this, but keep it in mind. If management isn’t willing or able to put engineering practices first, or to let engineers do their jobs the way they’re supposed to be done, it could signal other issues.
https://www.amazon.com/Working-Effectively-Legacy-Michael-Fe...
It's always tempting to use the word "rewrite" but most of the time you just want some assurance that your code doesn't break things, and a rewrite is usually overkill.
I would suggest you to add in additional time for refactoring into your new feature development. This has 2 benefits - you're not rewriting the whole project, just the parts that are relevant to what you are currently working on, and you're still pushing out features.
It sounds like most of your refactoring work is in code isolation / scope management and creating tests.
I had (well, I’m in the middle of it, so have) to fix a few bugs in a project that are the by-products of a deeply flawed architecture. I came to my boss and explained a handful of ways that I could potentially solve the problems, and made it really clear that the best way to go would be to completely redo this one part. He agreed with me, though he wasn’t happy about how much work was involved. ¯\_(ツ)_/¯ Hopefully your managers will appreciate you giving them options.
Take this one stage further, taking solutions to your manager is worth 10x taking problems to this manager.
1. Is this code in production already making profit? 2. How often is this code updated? 3. Is it profitable to fix that code or just to learn from that experience and move on?
Is it typical to be able to look at a company's code before signing up? After having a similar experience, I wish I could make that a standard part of my interviewing process. (Brief backstory: I would never in 1000 years have taken my first development job if I had seen the code first, but HR and the carefully selected engineers who could tell positive stories gave me a false impression of what was going on.)
We now have to demand to see the codebases we are going to work with/against first. Even if we only touch the API in some cases; if the API is built on quicksand then we risk spending a lot of time building something that is not viable but that might not be immediately clear.
I guess it is not typical, but at one company I interviewed for I had a day for implementing something (a small feature and some unittests) which allowed me to check their codebase. I liked what I saw and worked there for some months.
If the answers to those questions are no. Then you the problem and you should look for employment elsewhere. You are unhappy in that job, and you rewriting the codebase would potentially make the above questions answer to a YES, inflicting damages in the company.
Now, if the above questions have yes as an answer. Talk to your manager, explain how it won't scale, how it is hackable, and the bugs you found - and get the blessing to make things better!
Do not badmouth the previous maintainer/author you do not know the circumstances that led them into creating such a codebase.
However, that isn't what I meant by the word "problem". I was talking about end-user problems.
I like to think I'm pretty good at programming, development, and generally architecting solutions. I don't have a lot of experience, but I have a bit of talent and spent a lot of free time learning. Solving problems with computers is something I'm passionate about.
I started this particular project without any idea of what it was (okay, they told me what it was, but it turned out to be something completely different). When we did a kickoff it was myself, two other developers (who both said during the kickoff they didn't want to be on this team, and wanted to go back to admin'ing MSSQL databases), and a non-technical TPM (last time he touched a terminal was a decade ago, and he wasn't particularly interested even back then).
I ended up doing pretty much all the work. My PM would CC me on emails, I'd write the stories, he'd help prioritize them, and I'd do them. The first few weeks they wanted three services stood up and managed in an automated fashion. I got it done, it wasn't pretty. Hell I hadn't even figured out a DNS scheme, but this had to get done so other teams would be unblocked; it was the development account so I wasn't too worried about rushed work.
Then we got more requests from other teams, and vague requirements from the infosec and governance teams. Then the board that reviews new technologies shot down our (my) requests to introduce automation services like Ansible Tower. Then the AWS team shot down our request for multiple accounts, one per environment.
At this point the Cloudformation isn't scaling, I beg and plead for some time to freeze feature development and move to Terraform. I asked for 3-4 weeks, and was given 4 days. This was the one chance to move to Terraform though, otherwise we'd be using Cloudformation for the foreseeable future. So I took it, deciding I could rush through the Terraform rewrite, because then at least we could refactor it later during downtime.
Next thing you know a year has gone by and we're going into production. Everything on the devops end goes pretty smoothly. My velocity is crap though, due in large part because of the shitty rushed work, which it seemed was all I was capable of.
A few months after go-live, we finally get a real TPM, which was a godsend. I was moving to another team at this point, and so his first order of business was to replace me with two new guys. I spent a lot of nights interviewing and a month afterwards helping them get up to speed. I and the new TPM gave them cover that month so they could embark on rewriting the Terraform.
They ignored my suggestions on how to refactor it, which is okay, I was leaving the team, it was their code as now. But it was hard, hearing them laugh at the code behind my back, saying how bad it was. Never giving me a chance to justify it, or even reading the commit messages, to understand why it was done that way.
I'm off the team now, and while I recognize I left a lot of legacy code behind (which I'm not proud of), it wasn't just me who created it. It was also the business who said time and again development speed was the most important thing.
That's how one legacy codebase was created. There were a lot of things I might have been able to do differently (mostly politically), but I truly believe I did the best I could with what I had. But what do I know? My new TPM seemed to agree with me, but maybe he just didn't want to make an enemy. Perhaps I could have headed it off if I'd said the right words to the right people. I don't know, will never know.
This experience is why I don't make fun of others solutions. Somebody put effort and time and thought into it, and I guarantee (in most cases) I don't have that context to properly evaluate it.
that's assuming the original authors used version control, committed often enough to leave meaningful commit messages, and bothered to explain their decisions.
pretty much every code base i've taken over in my current role have consisted of a single posting commit with zero documentation.
And outside of the one place where I was the person to introduce source control to the organization, I've found commit messages to be pretty telling. Most developers tend to let their feelings pop out in their commit messages, and too when and how often they're committing.
In your particular case, sounds like either a junior developer, legacy systems, or a sole developer.
> Never giving me a chance to justify it, or even reading the commit messages, to understand why it was done that way.
This is a surefire way to re-introduce bugs that were fixed ages ago. Understanding why things are the way they are is the first order of business when working within a legacy code base. Was it done this way because the obvious way has a non-obvious edge case? Was it a requirement from the users? Or was it just the fastest way to do it at the time?
You have to have a certain mindset to effectively work on legacy code, and this isn't it. Most more junior developers I've known have a "rewrite" oriented mindset, meaning they don't know how to work within the constraints of what exists and refactor piecemeal. It's a hard to learn skill, generally learnt the hard way.
Open question: how does one fight against what's been described in this comment? What specific sorts of communication skills? One thing that comes to mind is learning when and how hard to push-back, and how to argue with force when necessary.
The reason I ask such a specific question is because I'm not great at confrontations. I don't dread locking horns, but I'm not good at it. The more force I tend to apply, the more crass and less refined (okay, rude/arrogant) I come across, entirely unintentionally. Diplomacy seems to me to equate to "die a bit more inside, burn some more of your own wick to deal with the frustration, don't say anything, and wait and see how things work out". So I end up not engaging at all. I know I'm missing something huge here and I don't know what it is.
I'm actually speaking completely generally though; I'm falling apart at this universally speaking. Perhaps I just need to learn how to debate?
When I'm presented with a context in which I need to prepare any sort of defense, 51% of my brain will sometimes go silly, and I'll get really anxious and irrationally fearful and tense up. Things rapidly go south from there; the biggest side-effect is that my memory completely shuts down, meaning that I cannot remember a) any of the things the other person said (so I cannot form an effective defense), b) any of the things I said (so if I say something incorrect[ly] I won't remember) and c) the way I say what I say, which means that my tone of voice might be off and I'll basically be too spaced-out to notice. The resulting arguments are almost comical in their narrow-focusedness, with the resulting back-and-forth many many degrees away from whatever the original point was (which typically gets lost within the first few seconds of conversation).
This is probably due to autism and some related undiagnosed anxiety disorder, but labels aside I've never really known where to start tackling something like this. I've given up on mental health support, which only has its own agenda (I don't want the brain-fog of drugs, so all the departments could care less), I just want to brain-hack my thinking onto the right track and I've no idea how.
FWIW, this is one of the reasons I'm actually reluctant to get a job, so I'm yet to get practical experience dealing with real-world issues.
Your recall may improve if you hand write your notes versus typing them out.
This would be worth its own Ask HN, if it hasn't been posted before.
Always loved this quote.
I'm not saying that you have an option in a do-or-die situation where it needs to be done in a couple days. I'm also not saying that you should subvert or defy management to execute a massive overhaul or total rewrite. I am saying that if you understand a system, know that it's fundamentally flawed, and have a window to fix it (even if it's a really tiny one), but choose not to, you are part of the problem. The standards that software developers are held to in terms of quality are an absolute joke, and the "it hasn't completely imploded our business operation, so don't even think about fixing it" mentality is where those standards stem from.
But what about:
- maintainability, extensibility, reusability, configuration
- mean time to detection / mean time to repair during an incident
- bus factor, cost of adding more developers, documentation
Code can still be problematic in terms of those and I think that would be closer to what the OP described.
Consider you have 2 cars: both are reliable, but during maintenance one of them requires you to buy rare and expensive replacement parts that you need to import yourself, only one shop in the entire city can work with it and it takes 2 weeks to get the work done. Which car would you rather have after going the process I described 3 times?
Chances are, the code is a mess because nobody who knew better was empowered or motivated to advocate for a better way, but you have a hidden opportunity to be that hero.
Good luck!
Do it.
Gradually.
Rename. Simplify. Reduce. Enhance. Test. Slice and dice.
Embrace the shitiness. Accept it. Own it.
It's your playground. Sharpen your skills, and lead them to the promised land.
It could be worse. You could face major resistance and people issues. Appreciate it.
This is an opportunity for you to kick a billion miles of ass...
Also see if you can make a to-do list. This will help make your request concrete to management. As you cross items off it will help them see that you're actually getting work done. And, it could help to motivate you if you feel like you're "just rewriting the code".
And if you have the option to rewrite it and you think you can do a good job, go for it and good luck!
This is easier to deal with both in terms of selling it to management and making sure you don't actually break the whole system compared to a full scale rewrite. I've made that mistake before and it wasn't fun. As others have said, code that's in production and works and is making money has value. The "cleanness of the code" is not something business owners care about, unless it causes problems.
I got it a while back when working on a similar sounding codebase and it was very useful. It's got chapter headings like "My Application Has No Structure" and "I'm Changing the Same Code All Over the Place".
If your company has a books budget get them to buy it, but I'd personally drop the £35 it costs on amazon given how useful this has been to me over the years.
Do you mean global variables? I guess it entirely depends on what language and framework you're using, but trying to refactor things to have less global state is generally for the best.
Back in the 90's and early 2000's, almost nothing had automated tests. Miraculously, things still got done by manually testing.
On the other hand, I worked at shops where every getter and setter had a test. Not much got done there.
That is still very normal and not very miraculously considering it happens all the time and very large companies run on this practice.
Although I am a fan of automated tests, I work with enough codebases that are robust and have been working for over 10 years that have no tests and others with a lot of test that are still fragile and miserable to work with. It is not all that black and white but you imply that as well with the getter/setter tests.
Edit: definitely kick the tires a bit by seeing if this situation is typical for the company, or if it's an outlier.
Regardless of the exact situation, there should always be a way for you to work on your craft. It sounds like you are doing a lot of refactoring now and that could be quite rewarding, even without tests. If you find yourself trying hard to improve the environment and feel like it is to no avail and that no one is listening, while simultaneously feeling like you can’t leave the job in the short term, you might want to think about how to take advantage of the dead time at work to learn new things or do other activities that improve your skills.
1. Try to get the political will to change it, then lead a change. This is essentially impossible unless you're a respected senior developer, and even there it is hard. There is a lot of advice out there, but the primary thing I've seen work is this: Complete feature freeze. API-ify the backend with heavy tests through the API. Have the presentation layer slowly shift to the new API. Once the API is complete, delete all the other backend code. Refactor the presentation layer with tests as you go.
2. Accept that the codebase is shit and look at your situation realistically. If you can't switch jobs (visa issues, etc) you need to accept that this is the way it is at this company and don't grumble about it. Spend 20% of your time making tools that help you compensate for the shit quality of the codebase; admittedly this is easier on non-Windows projects. Even so, making a small tools for disorganized projects really helps make you look super pro, even if the code is doing most of the work.
3. Quit and work somewhere else with better people and a better codebase.
Out of all the reasons I advocate TDD, from cybersecurity to QA, they pale in comparison to employee retention. Keeping a codebase clean over time requires tests. It just does. If you don't have tests things ossify. I don't write tests for ad-hoc querying data, but I always write them for long lived projects and I refuse to work on established projects that don't have test suites (or direction from on-high to add them).
Yes and no. Let's be honest, we all rewrite (i.e., refactor). It's just that we do it to our own code.
Long to short, this might not be what you enjoy but it has the potential to be a great learning experience. Doing code autopsies can teach you more than you realize.
No, absolutely not.
Look for the positives in what has already been accomplished, then sell them on the idea that "it can be even better."
Since it sounds like they are fine with you making improvements, just take it one step at a time and keep at it.
The code base that exists took many hours to develop. You wouldn't have a job at all if it didn't exist in some kind of functional state, no matter how bad it is.
Don't brag. Don't position yourself as some know-it-all hot shot. Just keep improving things and let your work speak for itself.
Ideally, you want to get good at explaining why the change matters in terms that make sense to the non technical folks involved in making decisions.
Do make sure they know you did the work. Don't let someone else get credit for it.
Here's what actually happens.
By that point in a project's lifecycle, that's not what they usually ask of the new hires. People are usually tasked with fixing bugs/maintenance, and implementing new business logic. Both of which are extremely challenging and stressful when working on a project that comes with no tests, or documentation.
Your actual task as the new hire is to start generating more value for the company, but that's so much more challenging when you have no way of validating your solutions, no infrastructure to rely on, and no metrics or documentation to consult. At that point, being the new hire, you're taking a lot of heat for not getting things done on time - mostly through no fault of your own. At that point, "wanting to throw you under the bus to your superiors" is a very natural response - it's a defense mechanism, you're (rightfully) frustrated. You're between a rock and a hard place. At that point you're no longer doing software engineering, it turns to politics/coverups/fire-extinguishing on a daily basis. Can you not relate to that?
The best advice I can give is DO NOT start a stop the world rewrite! and to look at the SOLID pattern and implement it using the STABLE strategy.
-Smell your code -Tiny changes first -Augment your tests -Back up -Leave it better than you found it -Expect good reasons
Full credit to Sarah Mei
https://markheath.net/post/stable-tactics-for-writing-solid-...
2. Relate your impressions to the person in charge: "The code doesn't seem to follow best practice and appears fragile. You think there may be technical debt that needs to be addressed sooner than later."
3. Until you're confident enough that you can pitch a comprehensive redesign and rewrite, wait for the person in charge to direct you towards specific repairs. When they direct you towards these specific repairs, they will anticipate regressions and instability. They'll see you as working through issues, not causing them.
4. Repeat step 2 periodically and with greater detail until you're ready to make the pitch in Step 3. Don't rush it.
The alternate solution: leave and let somebody else deal with the problem. Some people are comfortable working around fragile and wonky code; some people get really stressed out. You don't need to work in a job that's going to perpetually stress you out.
Yep, because I have worked with very bad codebases over the past 25 years, I actually enjoy the challenge. For me it is more rewarding than working with pristine and well managed code bases.
I was explaining greenfield vs. brownfield to a co-worker (non-developer) between periods of fixing a previous, junior, developers code that was essentially creating a seemingly infinite number of pages on our site.
Keeping in mind the above, I also know that deadlines, changing requirements, and lack of a clear goal (due to the previously mentioned items), lead to this. Had I not worked with them, my decade and a half of experience would have strongly suggested that such was the case.
When it comes down to it, some people prefer working on new projects, and some people prefer supporting and polishing those once-new projects. But to really succeed you've got to be able to do both, to some extent.
No matter how insane a code base looks, (unless it is totally inoperable) it is meeting a lot of conditions that are more important than whatever you are fixing but won't be recognized as important until you break them.
Those conditions built up over time and were changed irrationally by business decisions and hit or miss communication. A lot of demoralizing for anyone who was trying to keep standards while meeting requests.. Don't beat up the past maintainers for a past you can't know in much detail.
Maintaining old code can be fun though and is a more interesting puzzle game than most new development. Just try to stay emotionally neutral and unattached to anything about the code besides the specific challenges as they need tackling.