Hacker News new | past | comments | ask | show | jobs | submit login
Ask HN: Codebase at my work is a complete mess, what should I do?
406 points by anonR4nd0m on Aug 22, 2018 | hide | past | favorite | 333 comments
Recently I joined a relatively small company (50-100 employees) as a medium level software developer. For a while I was very excited about this opportunity.

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".




I've been on both sides of this issue in the span of my career. I've joined companies with absolutely horrific code, and I've hired people who thought my code was shit.

My advice is don't complain and don't attempt any significant rewrites for the first 6 months or so. Your job at first is simply to understand the code and demonstrate that you are able make improvements to it without breaking everything. Doing so will win you the trust and respect of your teammates, whereas if you come right out of the gate trying to rewrite everything there is a very good chance you will make yourself look like an ass by spending a lot of time and energy introducing bugs into a previously-stable area of the code. Once you have a decent understanding of how and why the code is the way it is, and the trust of your teammates, then you can invest effort into fixing the highest-priority issues with the code.

Being able to read and work effectively in legacy code is an essential skill for any software engineer. For now you should just try to focus on fixing bugs and implementing new features, and take pleasure in the fact that you are improving the project and developing a handful of hard and soft professional skills.

That being said - there should be a limit to how much bullshit you put up with. If after six months to a year you still find that you hate the job, and a better opportunity comes along, then go ahead and take it.


> Being able to read and work effectively in legacy code is an essential skill for any software engineer.

This is key advise. It makes the difference between a person who enjoys the art of coding and a professional who get paid for work. Basically you have to first proof why the invest of rewriting brings any financial benefit. Till then, be professional and maintain it.


Just to offer a different opinion here: I think happiness at work is the most important thing for me personally, and I have no patience for legacy codebases that look like shit.

I mean, you are offering good advice for a person that wants to fit in as his highest priority, no doubt about that. But there are people (like me) who don't enjoy working deep down in complicated shitty code to the degree that I don't even want to be a programmer in such environments.

There are ways to mitigate this. Join companies that care about clean code and micro services. Work in the cloud. Avoid enterprise. Those advices can be hugely important for someone who just hates those kind of gigs where you feel like you don't want to be there even.


I consider the ability to work with legacy codebases an essential skill. If you can’t do that, you’ll be changing jobs like other people change underwear.

Any codebase that sees real usage will accumulate cruft, odd behavior and legacy code over time. Dealing with that and assessing what’s worth refactoring and what’s best cut or even just left as it is to reach a business goal is the skill that differentiates a seasoned engineer from a code monkey. That shitty legacy code is often what brings in the money to sustain the company, otherwise it would have been turned off long ago.

Microservices don’t help much in that regard: each micro service can look squeaky clean from a code perspective and the whole system can still behave in shitty, unpredictable ways. (Been there, seen that)

It’s also not about fitting in: taking on a shitty codebase and transforming it into something better is an outstanding skill and requires constant communication with all stakeholders, especially when you want to cut legacy stuff. It’s certainly not something for people that prefer a quiet corner.


Are you saying you don’t change underwear often? ;)


Not every few hours and I’ve seen people quit a job between starting late on the first day and an early lunch break. So literally faster than I usually change my underwear.


These people should have asked to see a sample of what they were going to have to work on.


They didn’t even have a look at the code. Their task would have been to write and maintain end-user documentation and they left before they even had computer and stuff set up. Just never came back from lunch. Said they’d not feel up to the task. I was just a student at that time, so I can’t really say if that was a hiring fuckup or just an odd person. Certainly one of the weirder things to witness in my career.


Most likely explanation is that this person received a better job offer and did not wanted to let your company know.


>and micro services. Work in the cloud. Avoid enterprise.

How is this even related? Worst code I've seen was in financial sector, but the project is so huge that is no chance of upgrading it without complete rewrite, 65% of code when I left it was still in Java5 with no option to upgrade to 7 any time soon, it's just too big, so we can understand why it's shit. Second worst code was in a startup, micro-service oriented code, k8s, dockers, javaEE, angularJS and angularIO + TypeScript. There was literally shit, HTML injection in chat-box, >200 JSON endpoints with something that mimics REST (it was closer to CURD than to REST), SQL injection using JSON objects was possible, deployment of new version of a microservice usually took 4-5hours, only if any of model classes haven't been changed. Now I work on medical software, the only split we have is frontend has own repo, backend has own repo, two monoliths. Best code I've ever seen, perfect organisation, good class and field names, easy navigation. I see completely no relation between code quality and microservices/monoliths, what I see it that sane people write good code.

Sure, enterprise is slower with moving with technologies, but that's because they're bigger and decision on upgrading Tomcat to the latest version might cost thousands monthly if there is a single performance regression. It doesn't mean that code is shit because they still use jQuery instead ReactJS.

>Work in the cloud

How is that supposed to help? Moderns devs already have to know JSON, XML, XSD, 2 IDEs, 10 testing framework, at least 2 frontend frameworks and languages, at least 2 backend frameworks and languages, so here, have this k8s YAML configuration and this docker-compose and Dockerfile so you can... avoid shitty code? Also, have this microservice oriented stack, learn how to do distributed logging, autoscaling, self-healing architecture, deploy chaos monkeys in your cluster.


Thanks. Saved me a response :) I am in an Enterprise and we have deployed cruft code into the cloud :) and were even cloud native or whatever bullshit bingo brings up next in terms :)


Some of the worst code bases I have ever seen are cloud-based SaaS products. Including one that was bought by a large database vendor in the last 12 months for north of $US1B.


Aconex by Oracle?


Not going to name names unfortunately, especially not to an account created for the purposes of posting one specific comment.


People can make their own decisions about this, yes. But an important point I love seeing in a top comment is that almost always it's hard to inherit someone else's code. It almost always looks bad at first. I'm more of a manager than a coder but I've worked a lot of small places and I can tell you, every coder who has worked for me has tried hard to keep code clean, and every coder who's replaced those coders has thought the code was a mess. The only time this didn't happen was when there was really a team of coders working together. Any single coder is almost always going to produce code that looks messy to anyone else. You can say you're not going to work on code like that, but in my experience you're also choosing, in that case, never to work for a small place.


Well, that makes you a colleague who I would not want to have around. Someone who only cherry picks his duties and cannot work beyond greenfield projects.

I agree that work should produce happiness. And I agree there are places where the work is not everyone's fit (e.g. bureaucratic customizations). But being not able to cope with bad code is a no go in my opinion.


With microservices, clean code becomes largely irrelevant compared to other concerns such as logging, monitoring, tracing, API patterns, configurability, scalability, timeouts, retry logic, circuit breakers, etc. If the code sucks, you can just take 2 days and completely rewrite the codebase, whereas if the microservice doesn't adhere to 12factor.net, you're in for unwelcome surprises.

That said, any codebase that has seen a few years of maintenance and further development is going to look complicated, especially if you're too young and inexperienced to recognize that each of the annoying edge cases was put there to fix real production issues. Rewriting sounds like a good fix, but it will take a very long time and in the best case you'll end up with a newer codebase that looks just as complicated to a person just entering the business as the one you were attempting to replace.


I can appreciate this point of view but I think you are missing out. We learn as programmer's by making mistakes and experience. Familiarizing yourself with a crappy codebase (you didn't write) not only helps you get better at debugging but holds a wealth of experience of what not to do. As you don't write shitty code yourself, how else will you learn to deal with these problems? Everything we do should be to further our own skill set. My first real programming job was working with quants who wrote 1k single procedure Perl scripts, you had to scroll your editor to the right to see all the indentation from conditionals. When I look back it's one of my most valuable learning experiences.


Working on legacy codebases better designed than I could have come up with hasn't been much of an improvement for me. There is a certain quality of "habitable" for a codebase that tends to get tossed when people create a tightly abstracted object-oriented design. Peter Norvig and Richard Gabriel in their writings mention organizing software "fractally" from top to bottom, which I haven't really been able to figure out yet, other than it must involve some careful use of repetition. Apparently, it can help create those qualities.


Excellent advice, I agree 100% with this.

Also I want to add that allocating time for a refactoring or fixing legacy code is hard to justify most of the time. Instead I try to follow the "boy scout rule":

On every bug fix or new feature you work on "Leave Things BETTER than you found them". An small change like properly naming a class or a variable, or just writing a readme file explaining something will help you and your team in the future.

https://medium.com/@biratkirat/step-8-the-boy-scout-rule-rob...


+1


>> My advice is don't complain and don't attempt any significant rewrites for the first 6 months or so.

I would agree with this, you need to be careful. I've seen a friend start at a new company with a horrendous code base, bring up the issues and be let go because they hurt / offended the existing developers.

SOme people are really precious about their shitty code and can't take criticism, esepcially in small companies.

So get through your probation period then you can maybe start making noises.


> SOme people are really precious about their shitty code and can't take criticism, esepcially in small companies.

It's very easy to criticize, often not easy to understand the situation where the bad code was written. Maybe the hastily-written code was the thing that saved a company from bankruptcy and was written by dedicated engineers, and bringing your friend to fix it was the plan all along, and what they needed was someone to fix the code and not decry everybody as stupid.

[I'm not saying this is what actually happened to your friend. I don't know your friend :)]


> It's very easy to criticize, often not easy to understand the situation where the bad code was written.

A lot of people seem to think that criticism of the code is the same as criticism of the person who wrote the code. They are not the same thing. Root knows I've written plenty of crappy code over the years for very valid reasons. You are not your code, saying the code sucks doesn't mean I think you suck.


> You are not your code, saying the code sucks doesn't mean I think you suck.

Some people take _any_ criticism of their code personally regardless of how constructive you are being.

I have had a person completely blow up at me for simply asking (literally a question - a valid question) "why did you choose to use a BindingList instead of a List?"

Maybe it was how I phrased it? People are tough to get sometimes.


And some people are sick to their back teeth of listening to new devs come in and whine about poor code without understanding any of the context. Perhaps you do know better than everyone else, but you ought to make damn sure of it before you open your mouth


The first course of action if you find some ugly/bad code is to ask a dev with more history with the codebase why it's like that. In my experience 9 times out of 10 the answer is "we know, it's like that because of reason but we haven't had the time to fix it yet".


Surely in a commercial setting the primary factor is RoI rather than time? Will rewriting this old, stable, code increase profits or substantial reduce revenue impacting risks?

#notacoder


It really does depend, any code, especially bad code, is very rarely bug free. So if you ever need to fix a bug the amount of time you will need to spend and revenue you might lose during the time might be worth the refactor beforehand In my experience, software or product development is never really "done" Customers, users and business will ask for new features and change of old features, external dependencies and used services will change, hardware will change Not being able to move fast enough, train new engineers and adjust to changing requirements will have a cost If I can't trust my codebase not to break on any new change it will be much much harder and more time consuming ie expensive to maintain it

This is not applicable to every piece of code and roi should always be kept in mind but I'm my experience it is almost always the case


Same thing. The time is the investment. There are always projects with better RoI so a rewrite is constantly at the bottom of the pile. "We'll get to it someday."


Often, it's a simple case of business need. As long as the offices are clean, the business doesn't really care if the cleaning cupboard is tidy.

It applies to a lot of business areas. If you want to introduce new processes or fix any problems, you need to justify why you want to do that, and "This isn't very pretty" isn't generally considered a good enough reason. If something works, nobody ever got into trouble for leaving it alone.


> Surely in a commercial setting the primary factor is RoI rather than time

If it's a smaller company (op mentioned 50-100 people) they might not even know what RoI is, and/or might not care about it.


That's a strange comment, surely it's impossible to stay in business and not understand RoI to some degree.


Not necessarily related to code, more or less you just re-wrote a sort of Chesterton's Fence:

https://en.wikipedia.org/wiki/Wikipedia:Chesterton's_fence


I think if it's so frequent that you're sick to your back teeth of listening to people complain about your code, it's probably a really sure sign that something is very wrong with the code.


Haha, good answer :) It's rarely my code they're whining about though


> I would agree with this, you need to be careful. I've seen a friend start at a new company with a horrendous code base, bring up the issues and be let go because they hurt / offended the existing developers.

I'm not sure if I agree. As a programmer I feel my primary responsibility is towards the code, not the company, not my colleagues feelings, not the customer. There are other roles who speak for them (sales, product owner, managers, etc.), I speak for the code.

When you get into the office in the morning, you hang both your coat and your ego on the coat rack, you can pick them back up when you leave. If someone can improve the code by tossing away 3 years of my work, then by all means. Doesn't matter if the person who suggested it is a senior with 20 years experience or the fresh out of college guy/girl who started just last week. You judge the idea on it's own merits, not by who suggested it.

I really don't get the tendency of allistic people to involve emotions in parts of their life where it's not helpful at all.


I agree that ideas should be judged on their merits but you're still dealing with human beings. Someone could approach a bad codebase and say, "Here are some ways we could improve X, Y, and Z," but you could have another person say, "This was done in the stupidest way possible, we have to throw all this crap out."

If someone approaches things politely and professionally then no one should have a problem with constructive suggestions for improvement. But people sometimes approach things in a combative or disrespectful manner and that's when you run into problems.


> If someone can improve the code by tossing away 3 years of my work

But how do you objectively measure "improvement of the code"?

Maybe it's something fairly obvious like better performance, less bugs, etc, but a lot of times it's very hard to tell if there was actually an improvement or just a different way of doing the same thing, except at the added cost of re-implementing those 3 years of work.

And for the record, I agree with you in that you (we) should leave your ego at the door, and if some young whippersnapper can do better than you, then you should be able to leverage that and increase the productivity/performance of the team/company.

However I've seen it happen many times where someone thinks they will improve the code but in reality (like others have said) they don't have the correct context and forget about a lot of stuff that their "improvement" should take into account but doesn't... and then once they do the "aha!" moment kicks in and they realize why it wasn't as easy to improve as they first thought.


> When you get into the office in the morning, you hang both your coat and your ego on the coat rack, you can pick them back up when you leave. If someone can improve the code by tossing away 3 years of my work, then by all means. Doesn't matter if the person who suggested it is a senior with 20 years experience or the fresh out of college guy/girl who started just last week. You judge the idea on it's own merits, not by who suggested it.

>I really don't get the tendency of allistic people to involve emotions in parts of their life where it's not helpful at all.

I complete agree


> You judge the idea on it's own merits, not by who suggested it. > I really don't get the tendency [...] to involve emotions [...]

You're absolutely right: if someone has a good idea for improvement, it should be adopted regardless of who they are.

The difficulty is that the suggestion of a total rewrite is not-infrequently prompted by a mostly-emotional reaction. It may not be taking all the important objective factors into account, so you have to be careful to assess whether it truly is a good idea.


Disagree, joined a company 3 months ago. Started a huge refactor the first week in. Then, a bigger refactor the second month in. I feel I've already earn my credentials with these 3 refactors.

The directors and the team leader appreciate the honesty and my initiative to make the codebase healthier.

Like Ray Dalio says, 'don't tolerate shit'.

P.D: I rewrote a huge section of the application on Vue2 with SFC's. The other three developers who wrote backend mostly, tried to do the FrontEnd, but made a complete mess.


In the meantime, use an issue tracker to keep a handle on all the stuff you actually need to do to bring this code up to speed. If a test needs creating, add an issue for it. I'd personally not keep this in a tracker that your manager uses to track metrics, but keep it separate and build it up.

If it's really a rewrite job, you'll end up with a lot of entries in there of varying size and complexity. You can start moving those issues across to whatever issue tracker your boss does keep an eye on as you get to them.

That way, you're not just walking in, announcing that the entire lot is garbage, and wasting time rewriting it all (which is what the business will think you're doing if the current implementation is at least vaguely functional), but you're gaining familiarity with the code and compiling a decent sized todo list that you can present at the appropriate time, as well as a list of fairly quick wins that you can get done when you have some downtime.


I would add to this by adding tests gradually during this time. If there is no understanding from management that this is needed, do it guerilla style and do it anyway, maybe at a smaller scale.

In my mind, unit tests is not what you want to start with with a messy legacy codebase. It takes a huge amount of effort testing lots of already working code. Add unit tests for any new code, but try and implement tests for the entire system, functional/integration tests are also easier to argue for in terms for direct benefit for the customer. Unit tests are more for the developer and is harder to argue for.

Adding tests makes it easier to confidently refactor the code later on.


Any practical suggestions on how to bootstrap testing (unit and/or UI) for an existing project that has never seen any? Or any recommended resources I could read up on?


Particular resources and guides are gonna depend heavily on language/frameworks/etc.

Start with areas that are the most valuable to the business. Not only does this more easily show the (business) value of testing, but it helps keep you from introducing a very visible and very bad looking regression. Anything dealing with money (especially billing) is probably a good indicator. Be warned, here be dragons.

That being said, don't verify your testing setup with business critical tests. Verify your setup (to yourself and other devs) with the quickest thing you can accomplish. The business critical ones are your (team's) short term goal.

Pick out the X most important logic paths (e.g. customer checkout or some subsection of that). Work on regression testing that over the next few months.

Make sure your tests are both automated and actually get run. The latter is more important than the former.

Ideally, your tests will prevent a nasty bug from going in and will immediately prove the business value of your testing efforts. This will establish the trust you need to continue testing and refactoring.


Hmm.. it's definitely tough to begin on a codebase with 0% coverage.

The first step is to stop the bleeding so begin adding unit tests for any new code you write and be sure to factor this effort in with your sprint estimations.

In parallel work top down with high level integration tests written against feature flows. E.g., As a user when I select this then I expect this. This doesn't mean you need to actually use BDD gherkin library for your test runner but at least frame the context of these in that regard since you don't care about the minute details of the underlying code, just the experience of the user story.

And lastly, don't just write tests to write tests, try to understand what is important to cover against because there is nothing worse than maintaining barely useful tests wired up to legacy data fixtures.


https://www.amazon.com/Working-Effectively-Legacy-Robert-Mar...


I agree here, but keep copious notes. I've found mind mapping software, like freemind is good for this. It will help keep you sane.

Without that, IMO trying to internalize large amounts of badly written code can negatively affect your brain's ability to come up with good, well written code. All those bad ideas and patterns become the first things that pop into your mind when you try and solve problems.

“When you stare into the void, the void stares back into you” sort of thing...


Do you have any example of how you use mind mapping. I've tried a few times to use it but have yet to find a good way to layout my notes/thoughts in a map.


I don't have any hard and fast rules, but if it is a big complicated project I don't usually try and come up with a single coherent diagram of my thoughts. Instead, I start out with a node for the date, and a child node for a subject, and child node of that with a sentence or two concerning a topic I'm working on. If there are more details to it, problems, questions, etc. they become sub-nodes of that, and so on.

I don't usually change the details of nodes that I already worked on, but rather write what has changed in a new node with the same name but under a later date node. I find that it becomes a mess if I try and keep every node updated to what I currently think about it, because too much information builds up to efficiently do so. Also, a lot of what I write only make sense in the context of what I wrote on other subjects during the same time period, so if I change one, I end up having to rewrite more and more, which eventually becomes a big mess.

The main benefit is that I can go through my notes quickly, skipping minutia of topics I'm not interested in simply by not reading the details of a node. I used to use a text file, and that was a very big problem, because not only would the amount of text that I'd have to go through became daunting. Also, I sometimes found it hard to interpret what I wrote, which means I wouldn't know if it applied to the problem I was working on or not.

However, in contrast to the above, I do try and make a definitive structure if it's a small topic or a quick idea for a new project or something simple like vacation plans.

BTW, it also works very well for taking notes for a class or self-study.


One way to deal with it is to understand why the mess in the first place. You may find reasons to forgive the mess. If you're looking at a startup that had to try a lot of things perhaps (and when the team was junior) hitting on the workable idea later, maybe they just did what they had to do. The forgivance may also give you the incentive to improve it. A good measure is whether the shitty code does something worthwhile and interesting. Might get you to a point of empathy.

Edit: "you" = OP


That 6 months is also a great opportunity to establish a test suite. It's dangerous to tell management that the code is bad. It's not dangerous to say that it needs tests, since that's an industry best practice and there are objectively no tests, and that you believe that a certain amount of your time needs to be devoted to paying back some of that testing debt. Then, when 6 months rolls around, you either have the confidence to do a major refactor/rewrite or leave and give the person who comes after you the test suite necessary to do the next step.

Plus, to my mind, there's no better way to start understanding a new codebase than to start with tests, either writing them or reading the existing suite.


To add on to this, you can still improve the code quality while building on it by adding tests as you go, so you can have some solace and also sanity when testing your own code.


> My advice is don't complain and don't attempt any significant rewrites for the first 6 months or so.

Even if I didnt do anything, the code keeps breaking!


I came to this also. In first months of work is learning process: practices, workflows and processes. Then you should introduce changes via retrospectives. The same solution for outsource companies, that joined to old project. There are no time for such thing as debates which eggs are better. It should be introduce smoothly.


excellent advice.


Are you working at my company?

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.


If you’re going to take this to management, I would not bad-mouth the original developer.

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.


This is a great reply, and I want to second a lot of the points brought up.

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.


FYI, the Legacy code book is: Working effectively with Legacy Code by Michael Feathers. Its useful, I also strongly recommend it when you're feeling overwhelmed by a large sprawling code base.

https://www.amazon.com/Working-Effectively-Legacy-Michael-Fe...


>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

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?


I think it's quite rare for the prototype to not end up in production. This is even true of the Trans-Siberian Railway [which meant people died].


This is good advice, especially on the communication part.

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.


> Then give them a few options with trade-offs.

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.


> 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.


For example I work with a codebase like this. It is over ten years old, have multiple people working on it, many learning or of differing abilities. Frameworks have been used and over time their limitations come to light. Often people have been band-aided something to save time, because they have been under severe time constraints. Large swathes of it are almost superseded but not quite. The purpose of the software has subtly shifted over the last decade.


Here's a hard earned tip. The first few months, nobody's going to expect much from you anyway. Try to stretch the time you're 'getting up to speed with the code' as long as possible. But in that time, write as many tests as possible. Don't tell anyone unless they ask; if there is no testing culture, they'll think you're just wasting your time on it. Then at some point you will be expected to start delivering value. At that point, you can start making changes (refactoring where necessary) without fear. This will let you grow your status in the organisation until you have a reputation of being the guy that can get things done and doesn't complain about how bad everything is and how everything is someone else's fault when he's assigned a task. At that point, you can start making others write and maintain tests while you take control of keeping oversight of improving the code base quality.

Do not be tempted into 'rewrites' because you're setting yourself up for failure.

It's a long play, yes. If you're feeling the way you describe just from the current state, you're probably not senior enough to do it. It requires much more than just technical skills - those are abundant, it's not what you can build a career on. It's a bit Machiavellan, I guess, but in the end, everyone wins - but you need to have the Vision that others are not able to understand, so you have to find ways to make it happen anyway, in adversarial circumstances.

Good luck.


If there's no testing culture it's pretty hard to introduce it into a team that doesn't have it (either by choice or omission). It gets even worse if your teammates don't update tests (thus break CI) on code changes or just comment the tests out. I've seen both happening way too often and would recommend getting at least some sort of team buy-in - as the alternative will surely burn you out sooner or later.


If you're the new hire and the first you you're doing is pester everyone with extra work they don't see the advantage of - that'll just get you the reputation of a know it all, in my experience. That's why I suggested to start chipping at it on his own, so that he can show the value a few months down the line, rather than telling everyone how great it's going to be when he's not respected yet or hasn't proven himself yet. But that's also what I meant by 'it's not a technical issue'. It's a matter of positioning himself as a reliable person whose opinion on technical matters needs to be considered. New hires generally aren't, unless they have an impressive resume or reputation before they started at the company. But someone like that doesn't ask this question to a bunch of strangers on the internet.

Of course it depends on the circumstances. If you're on your own trying to maintain a test suite on the code of 50 other people, none of whom work on these tests and think you're just trying to slow them down for no good reason, there's no way that will work. But if there's only a few people working on the code and you can maintain tests for a more or less well defined part of it - it's possible (if you're experienced enough; if you have to spend 2 weeks reading up on how to do unit testing, it's not a viable strategy of course.)


I've actually seen it done within a large organization. It takes one boyscout who's respected and can clearly demonstrate the value provided by testing and designing for testability, and people will fall in line. Nobody likes building on quicksand, so show them what concrete looks like.


> Nobody likes building on quicksand, so show them what concrete looks like.

Damn that's motivating. I'll be quoting you on this one.


If you have the choice, change teams or companies as soon as possible. Life is too short to spend it paying off someone else's debt.

If moving is not an option, then follow the parent's advice. Add all the missing tests as fast as you can. Explain what you are doing to your manager. And set up CI to email your team whenever anyone checks in breaking changes. This will motivate your teammate to start running your tests. If they are particularly narrow-minded, they will be unhappy about this. If this happens, reconsider your decision to stay.

Even after you write a lot of tests, coverage will be low. You will likely code some bugs that negatively impact the business. I urge you to find a mentor inside the company, a senior engineer or engineering manager, and show them code and explain the technical debt to them. Try not to blame your teammate for the situation, just explain to your mentor that you are concerned about introducing bugs. Ask them to keep your discussion confidential. Your mentor can back you up when your bugs negatively impact the business.


> write as many tests as possible

Definitely: this will help you to better understand what the code does and the dependencies involved. Climb up the dependencies tree and add tests there as well.


So you're the new guy that's been assigned to this project...possibly at the request of the current developer who desperately needs help...possibly because it's the project that existing staff don't want to work on...possibly to learn the project so the current developer can be moved to other projects.

Every software company has these projects and many developers have been in your position. As long as the software is still functional and is profitable then management don't care how bad things are under the hood (try explaining it to someone who only sees a working/functional UI). The fact a new resource (you) has been assigned suggests it's still a valuable project.

Unless management are the ones giving the go-ahead on the rewrite then I wouldn't bother. Rewrites are very risky, time-consuming and make no money for the company. However long you think it will take...double it. You're a mid-level developer, a rewrite needs to be planned and led by an experienced senior developer.

Your best bet might be patience...try ride it out and hopefully be moved onto other projects. In the meantime you get experience. It's how you handle the adversity that could prove more valuable.


> Rewrites are very risky, time-consuming and make no money for the company.

It can also put you in an awkward position -- especially if the prior developer has a grudge against their code being rewritten -- as any bugs that come up can then be blamed on the rewrite effort, whether or not that was actually the cause.

Spaghetti code eventually collapses on its own, as every change starts taking longer and longer than expected, as well as introduces more and more bugs (and fixes for those introduce more, and so on). Once this starts happening, that can be the impetus to actually do a real rewrite effort, but one with a business case, management and hopefully the other developer(s) behind it.

Until then, a rewrite is a lot of effort that, at best, if you do a great job, is basically invisible (beyond the code level), and at worst either becomes a scapegoat for or the cause of many other problems.


Agree with all of this.

I don't work at a small company but refactoring and especially rewriting code is difficult and error prone. It's a crazy thought but in many cases, it can be better to just leave the legacy code alone. It's all about ROI of your time and the opportunity cost of what you're working on.

If you're going to be making lots of changes to a specific part of the code base, then it very well may make sense to invest in improving it so r hat you can move faster after. But if the code isnt going to be modified much, it may be better to just get in, make the changes you need, and get out.

As a mid level developer, nobody is expecting you to come in and rebuilt the thing immediately. Focus on learning it well first.


Rewrites are riskier when requirements need to be reverse engineered from the current solution, or if requirements are complex. When requirements are simple, rewriting should be simple.

This should at least give you the opportunity to contain technical debt to the most complex requirements.

To say "everything is complex therefore we cannot rewrite anything" is pretty implausible and mediocre.


I disagree. Rewrites are risky because you're changing code that has been working for some time. It is easy to introduce bugs into a sufficiently complex system when refactoring.

"The idea that new code is better than old is patently absurd. Old code has been used. It has been tested. Lots of bugs have been found, and they’ve been fixed. There’s nothing wrong with it." - Joel Spolsky


parent is right in that when you realize there's significant functionality that is not captured in the requirements, then it's it's good idea to NOT REWRITE... when you end up reverse-engineering requirements from an existing app, you're standing near a bottomless pit of other horrors you can't even fathom... that's not gonna be an only 200% miss-etimate! think in the range of 10x (order of magnitude) difference between actually and real effort, and how many corners will need to be cut and personal time sacrificed to get from 10x to a "management acceptable" 2-3x overtime during the rewrite.

You don't want to live in this hell during the rewrite, trust me!

(Oh, and another tip: if your management authorizes the rewrite you propose easily, it means you are SEVERELY UNDERPAID... the time of good devs always costs to much for rewrites to be worth it, unless the dev cut a really bad deal on hiring)


I doubt Joel Spolsky was equating legacy code with technical debt in this quote. Not all technical debt is robust and well tested legacy code.

Chromium for instance has code from the original KHTML written in the 90s, most of that remaining code is reasonable and makes sense today. If it didn't someone would have replaced it.


> However long you think it will take...double it

... and also don't think that your "double of original estimate" is anywhere near a maximum of what it can take to actually make the code base better and functionally correct.


> functionally correct.

It is often true that someone with very strong domain knowledge may have had this project in their head for a long time, and one day, all-of-a-sudden, has to code it all in a rush. I have had this, where external factors shifted the companies priorities.

In this case you can 'brain dump' a lot of code very quickly, get it functionally correct and make intelligent judgement calls about the threats of edge cases... especially if it is internal company software.

However, coming at the problem from cold, without the benefit of this knowledge requires significant analysis and engineering effort to get equivalent results. So tread very carefully.

But yes, add tests


Somewhere by someone I read that a realistic estimate is your original estimate multiplied by 8. This has proven to be more realistic than previous factors I've seen.


Increase the time units by one and double it.


> The fact a new resource (you) has ...

I am not familiar with this lingo. Why exactly is the OP a resource? Is this usage of "resource" common in business speak?


Yes, developers are resources, what they produce needs to be of enough value to make up for the pay you supply them with. Which is why refactoring and rewriting is almost never on the agenda for management, because it often doesn’t bring in value.

In rare cases a piece of software is so bad, that rewriting it makes sense, because it means you’ll save development resources in the long run, but in my experience that’s rarely the case.

The best developers write good clean code with low dependencies and tests for things that matter, most developers aren great developers though, and things can almost always be made better, even if they are.

So it’s really about time vs results. A good example is RPA, it’s some of the most simple code around, but it’s often very hard to get it to take over 100% of a process. Your developer spends a few days to take over 80% of a manual process, but the last 20% would take your developer months of small adjustments.

That’s often not worth it, and refactoring is often the same.

If the system isn’t slow, and you can still add new features to it without breaking anything, then it’s not worth a refactor.

This doesn’t just apply to developers by the way, I manage a lot of different types of people, but they are all resources that I need to fit within my budget to meet the goals given to me by my management, who in term see me as a resource to meet their goals. We’re all cogs in the machine.


Yes. "Human Resources" is quite literal. Management discuss allocation of resources (i.e. engineers) to teams, open REQs (requisitions) for resources, etc.


Where I work, "Human Resources" is just in charge of benifits/payroll/etc. Assigning people to projects is done by the resource manager, a completly unrelated posistion.


I manage my team. When I discuss projects with my manager and the senior engineering leadership, they will almost always refer to staff as resources, e.g. "team X needs a SOA resource", "team Y needs extra QA resources" and so on. It's distasteful to me; I always refer to my team and other engineers as engineers or by name, not as "resources."


To balance things out, try casually slipping in the word "overhead" when talking about management.


I do that. Does not harm. They know themselves :). Good advice!


By the 'resource manager'. Point stands.

But, no place has HR assign resources.


It is a term to enforce the concept of humans as commodity in a commercial environment by the overhead.


That's pretty normal in the industry.

Had a client where the employees where stored in a table called "resources". Also felt very much like a resource there..


Yes. It (being called a resource) is common in Big companies where they literally see you as a "resource". This is worse when you are a 'resource' in a big service company.


No it is not. That is just project management language.

It is the culture of the company who makes the difference.


More precisely, your labor is the resource.


Swap out the word "resource" with "asset" if that helps. They're synonyms in this context. "A useful or valuable person."


Unfortunately, yes. We are just replaceable cogs.


I'm okay with this. They are paying me because the work isn't something that I'd do in my free time. I'm okay knowing that others can do my job as well as I can.

Its not art, it's a job. (to me)


I'd rather be a replacable cog than a crucial lynchpin. I prefer to look at my job in the same way. It's much less stressful that way.


Are you familiar with the term "human resources"?


What do you think HR is abbreviation for?


Don't fix things just for the sake of fixing them. Let new features, bugs, or necessary performance improvements drive your changes. Keep your changes small and focused. You need to ensure that you're delivering business value with each change, rather than embarking on an overwhelming, open-ended task.

This is critical if you want the business to support your effort. It demonstrates pragmatism. It will shrink the scope of your changes and make the problem less daunting. You can prove to your boss and your coworkers that it's possible to fix the problem without slowing down new development efforts.

Be careful how you express your dissatisfaction with the code. Many of the people who wrote it are likely still there. Approach every change from a positive angle, so you don't alienate your peers. Get them to help you make the changes, and help them to learn how to become better developers. Don't assume they don't know better; they may have inherited a mess too, and they may have been under pressure to deliver quickly at any cost--or maybe they just don't know any better, but want to improve.

Read these books:

_Working Effectively with Legacy Code_ by Michael Feathers

_Refactoring: Improving the Design of Existing Code_ by Martin Fowler

_Clean Code_ by Uncle Bob

These are invaluable resources. There are many others, but these three are particularly germane to your situation.


I would agree. Your job is to implement the new features and leave the code in a better state than when you found it.

As a professional programmer, your job is not to produce beautiful code. Your job is to make money.

There is a commercial decision for the organisation to make about paying back the tech debt in this code base. That's not your decision to make. Likewise, it's not your decision to create more tech debt if you can avoid it (sometimes you can't because deadlines are more important than tech debt).

As the parent said, your best course is to implement the new features, and refactor where it makes sense to do that to support the new features. But without any test suite, you risk breaking the existing features by doing anything more than the absolute minimum of refactoring. That's bad.

Good luck. And welcome to the profession hehe


I don't agree with the first part. I have worked enough time on the management side to understand that management is always driven by business value (it's their job). So your point 'Let new [...] drive your changes' is valid to some extent. But business value doesn't always mean that you need new features to justify working on a project. Sometimes it is possible to justify working on something just by explaining technical debt and how much time it will cost in the future (e.g. slow you down).

Working in an unstable environment can drive you crazy as a developer and lower your productivity enormously. So if you don't talk about the issues in the code base with the management, they might wonder why you have lower output then others and classify you as a slow programmer.

Instead, I think you should bring the facts to the table (slow down factor (number of bugs, time to trace them down), risks (likelihood of production issues)), in terms of how much time it costs to work within the current code base and let the management decide on how to proceed (make sure to bring a recommendation). If they are willing to let you spend time on improving the quality, try to modularize it so that if the rewrite will be stopped at any time, the time you spend refactoring it helps you in the future. While doing so, show respect to the programmer that worked with that code base in the past (even if she/he create that mess). You don't know under which conditions she/he had to work, how she/he learned to programme or what her/his life looks like outside the job.

The management basically has these options:

1. just minor changes to the code base: no rewrite, no hidden quality enhancements (might be a valid option if an alternative software is being developed already). 2. hidden improvement: no rewrite, but you are allowed to spend some extra time to improve the parts you are working on. 3. restructuring of the code base and systematically writing tests for different modules (recommendation). 4. complete rewrite: unlikely to happen unless it is cheaper than 2 and 3.

If the management decides to continue with option one or two, do the best you can under the circumstances be pragmatic as caymanjim wrote. Make sure to include option four so that the management sees the full spectrum of options at hand.


Just a quick note: Martin Fowler has been working on a second edition of _Refactoring_, and IIRC it is expected to come out this Fall. See https://martinfowler.com/articles/201803-refactoring-2nd-ed....


I came to recommend Feathers' book. Those strategies will save your bacon when you go to solve these problems. I put it in the "must read" list for all mid-level engineers.

To be fair, I put all three of those in my must read category.


Bookmark this page so that 15 years down the line in your career you can reflect on it. You will have delivered a working software product against all odds, under impossible budget and time demands. Finally they let you hire some help and instead of getting busy helping writing tests and refactoring he starts thinking about the best way to throw you under the bus to your superiors.


> "Finally they let you hire some help and instead of getting busy helping writing tests and refactoring he starts thinking about the best way to throw you under the bus to your superiors."

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?


This should be the top comment. At my company it is amazing how clueless young graduates play politics from day one, while doing no real work other than going from conference to conference and self-promoting on Twitter.


Start writing tests, change no production code. You need to understand how it works (by reverse engineering its behavior with tests you’ll learn this) and you need to verify your changes don’t accidentally change existing behavior unless the existing behavior is verifiably wrong, and hey, you may find instances of this too, but don’t start making changes until you have a suite of tests that can spot behavior changes. There are times when things that appear to be bugs are actually weird features that the business needs to function. Sometimes cruft is actually a bug fix.

Start with high level integration tests of the most critical business processes. Then, work your way down to frequently used functions/classes/modules. After the whole thing is covered you can start cleaning/repairing, and I’d suggest doing so incrementally/as you’re adding new other features (make sure all new features come with tests).


I once did a major cleanup at the start of my career. The above advice is closest to what proved successful to us, together with the response by JohnBooty https://news.ycombinator.com/item?id=17825093.

In our case, I think we didn't write a lot of tests, because we didn't know they're so useful then. But much later I did a few ports of FOSS software between programming languages, and comprehensive test suites were absolutely godsend in those cases. So, if you can, try to write tests as first thing indeed.

Other than that, we took a kind of "organism fighting a cancer" approach. In a spaghetti codebase, we tried to find some smallest possible islands of the most isolated code (a lot of detectivistic work). Then try to improve the isolation even more — slowly engulfing the "tumor" in a more reasonable API. Once we had a somewhat acceptable API, with mostly well understood semantics, rewrite the internals of the tumor from scratch. Rinse and repeat.

Notably, as mentioned by JohnBooty, we did talk this with management. It made our product release late by ~1yr. The argument was that there was a particular critical bug, which would result in lost consumer data. This one bug was found very close to original release date, and opened our eyes to the horrors in the codebase (actually, a big part written by a subcontractor).

Sorry if the reply is somewhat chaotic, I didn't have much time to write it.

EDIT: Also, I believe the term "technical debt" is reasonably good when talking with management/owners, as it has correct connotation of an investment at early stage, which however has to be repaid later at a cost.


Definitely this, though I’d say the first thing to address is good observability. Are all errors being logged, with stack traces? If it’s a web service, are errors rates and timings for all endpoints tracked?

Next, the deployment process. Are tests run on all deploys? Are there canary deploys to minimize impact when bad code goes out? Easy rollbacks?

After that, I’d address the dev environment. Can you run it easily locally? Can you easily attach a debugger?

After that, add end-to-end, integration, then unit tests, in that order. Only then, once you understand the code well, and have it nicely instrumented and tested, should you consider significant refactors. By that point, you may even realize that a lot of it is good enough and rarely touched, and doesn’t really need to be refactored.

If any of the above steps are already well covered, skip them. And don’t spend all your time doing this - talk to your manager(s), try to get buy in to spend say 1/3 of your time on the health/quality of the code, 2/3 on features/bug fixes/etc. Justify WHY - in this case it sounds like the code would be hard for anyone but the original author to modify with confidence, to make sure the code scales past this one person without the product becoming full of bugs and instability, it’s necessary to devote significant time each sprint (or whatever) to improving quality. Can frame it as “original author was just trying to ship quickly, and could keep track of all the hacks because they wrote them, but to bring more people onto the project it has to be made safer/easier to modify for non-experts”. Maybe wait until you’ve been there a month or three before you start pushing this.

At the end of the day, the code doesn’t have to be perfect, it just has to be decently easy to modify with reasonable confidence. Tests should catch most errors, monitoring should quickly catch the rest to minimize the impact. Work towards that, over time, without killing yourself trying to sneak in refactors that your manager(s) don’t know about.


I think you are right, though in my experience in these situations is that defining the exact behavior is very hard. In not many cases was the messy code written messy from the start, it's usually an accumulation of changes (to behavior) - which are very hard to figure out.

I've found it's also hard to test code that isn't modular and split up: before you can test anything you need to mock 2 database connections, one external API endpoint and reading/writing from 3 different folder locations. Either that or you split them up yourself and risk breaking something.


So for a project that lacks tests, I’d suggest higher level integration/smoke tests so that you don’t have to modularize in order to write tests. Don’t mock a database, use a “real” test database with fixtures, if necessary use tools like Selenium and snapshot testing. These tests may be brittle and they may not cover all branches. But you’re looking for things like “can a user register”, “can an order be placed”. For http you can use tools like Charles Proxy to record / replay network traffic from real observations of the existing code running.


This is what i would suggest as well. I did some "rewrites" in my carreer and they only really went well when i worked long enough with the legacy code before. Essentially getting a feeling for what it really does, being that with tests (preferred) or by reading the same lines again and again while introducing new features/bugs.

If you feel the code is messy it is likely you are missing some essential business logic as well when blindly rebuilding it.


There's a book called "Working Effectively with Legacy Code" by Michael C. Feathers which I heartily recommend.

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.


Yes, so good. Martin Fowler's Refactoring is an excellent companion to Feathers' book.


It will be worthwhile to know that a second edition of that book is expected to come out shortly.


Is there a chapter on what to do if your coworkers don't initialize variables? I just inherited a steaming pile of legacy where they didn't and I'm struggling to understand how they got it to run at all. At least I'm starting to understand why we held off on the spectre patches.


Not explicitly, no, but it does have some techniques for reading code that might help?

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.


This book was written for OP (and many others). It specifically addresses the "there are no tests, what do I do?" concern.


Is it buggy? Is it insecure? Is it slow?

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.


If you ask me, this is a pretty awful approach. "Bug-free", secure, and fast are not the only metrics of whether or not a codebase is worth keeping alive. Some codebases are so obscenely dangerous that you really shouldn't be giving a shit whether or not it "works", because it's only a matter of time before it doesn't. There are code practices that are inexcusable under any circumstance. Sitting on a fucking nailbomb when it's in your power to change it because you don't want the headache of having to push for a real solution will catch up with you in the end.

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.


This seems like a great post to vent a little bit. I'm definitely a bit touchy on this subject.

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.


I feel like this is how most legacy code bases are made.

> 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.


Thanks for sharing this. Venting is good, and in this case a win-win, since this is good info (very relevant in a thread about encountering "bad codebases"). Hopefully OP sees it. You could well be "the other developer" where OP is.

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.


You really can’t, it’s not productive to assume this a skill you need or is beneficial (although it can be) and if people pick up your code and joke, laugh, or belittle without context, as in this case, it’s not your fault, don’t let it effect you. You’ll find the vast majority of developers are childish and immature, and not very good at their jobs, that’s been my experience at least.


Right, duly noted.

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.


Make/take notes. Sometimes the act of writing the notes helps your recall later. If you feel that you still can't recall your notes from memory, then refer to your notes and ask to delay any debate about the topic until you have them in front of you.

Your recall may improve if you hand write your notes versus typing them out.


> Open question: how does one fight against what's been described in this comment? What specific sorts of communication skills?

This would be worth its own Ask HN, if it hasn't been posted before.


> reading the commit messages, to understand why it was done that way.

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.


Given the context, I think the individual that you're quoting said that because they did do that within the commit messages.

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.


You have identified 3 important non-functional requirements: robustness, security and performance.

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?


A software project can be fast, reliable, and secure while also being extremely poorly engineered and extremely difficult to maintain and extend.


Not a large one.


"There are two ways of constructing a software design: One way is to make it so simple that there are obviously no deficiencies and the other way is to make it so complicated that there are no obvious deficiencies." — C.A.R. Hoare, The 1980 ACM Turing Award Lecture

Always loved this quote.


Somebody fathered a legacy codebase.


Only successful applications have the pleasure of getting old.


"Successful" by literally any measure.


I'm not sure about "any" measure, the average startup probably has no surviving code in production.. Even the average in acquired startups, if we count acquihires..


Success, in this context, means having solved some problem for a meaningful length of time.


That doesn't exclude anything. Code can exist to pad resumes, feed paranoia, and bolster egos, manipulate people, or simply because someone can't admit their own mistakes. Anything can be a solution to a problem.


Yes, those things do happen.

However, that isn't what I meant by the word "problem". I was talking about end-user problems.


All code is legacy, the only code that’s not is the code that’s not written.


So you're saying they speak from experience and know whereof they speak.


Why can't you badmouth the previous developer? Sometimes they were simply shit developers and wrote large quantities of shit code. Circumstances be damned, I've seen a few developers that should outright find a new career.


Generally because once you start badmouthing a person, it quickly gets into politics rather than being a technical discussion about the problems you're trying to solve. And with politics it's generally less about what you know than who you know, so the person who's had a chance to build connections in the company will probably come out on top.


[flagged]


Hard to guess their motives but it is easy to click on their username and see that they post a lot from it.


1. Stop "repairing" it because you're guaranteed to introduce bugs and regressions. You haven't learned it well enough and can already sense the fragility, which is why you're anxious.

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.


> Some people are comfortable working around fragile and wonky code

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.


Ditto.

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.


I love your answer because it calls out the perception of what you're doing. It's so important to frame what you're doing in a positive light, otherwise you'll fail, whether or not you make the codebase better.


Every programmer that has ever existed has said and felt this exact thing. It used to really bother me as well. Then I realized that the world is a messy place and I would need a stronger stomach if I was going to watch the sausage being made. Don't refactor because it violates your delicate sensibilities about code - that's a great way to waste a lot of money and get yourself fired.

Greenfield development or maintaining a legacy code base that isn't complete shit is a luxury. Leave things better than you found them but you have to earn respect/influence before you can start attempting sweeping changes.

PS - https://blog.codinghorror.com/the-ten-commandments-of-egoles...


Ha! Only tens of thousands of LOC? I've worked in places with millions of lines of spaghetti code. The reality is that most business software that's been around for a while looks like this.

If the environment itself is good, and you like the people you work with, then you should see it for the great opportunity that it is; learn how to slowly evolve a large software project for the better. There's none of the stress of trying to figure out the "right" architecture, no worrying about making bad decisions. You can just chip away tidying things up.

Things that are probably a bad idea: Trying to rewrite (will almost certainly fail), Blaming others (it's no one's fault, it just is what it is), Walking away (negative signal for future employers).


First start writing automated tests, for example Selenium & unit tests. You'll learn what all the code does, you'll learn a new skill, you'll have a safety net of regression tests for rewriting the code, and you'll be adding value to the company. If you have good test coverage, you'll be able to rewrite the code very quickly. There are a ton of resources for how and why to do testing. One that stands out in my mind is SQLite. They've talked about being able to refactor the core of their engine in a weekend, only because they had the test coverage to know they did it right. https://www.sqlite.org/testing.html https://www.youtube.com/watch?v=WslnuNZ3T5w https://youtu.be/giAMt8Tj-84?t=32m35s


Go buy a copy of "Working Effectively with Legacy Code" by Michael Feathers.

Start there. That book changed my career. Just look at the chapter names here: https://amzn.to/2wnPHfi

Chapter 6: I dont have much time and I have to make a change

Chapter 7: It takes forever to make a change

Chapter 13: I need to make a change but don't know what tests to write

Chapter 16: I dont understand the code well enough to change it

Chapter 17: My application has no structure

Chapter 22: I need to change a monster method and I can't write tests for it

Chapter 23: How do I know that I'm not breaking anything?

All the stuff is gold. It applies to every language, every work environment and every client I've ever been at.

This is the #1 book I have my teams read.

edit: Formatting and addl chapters


I am a senior dev managing a large and relatively messy codebase. Let's see if the way I read the situation checks out:

> I've already made some comments about rewriting it and the response was basically "ok".

This means that the person in charge of the code is probably aware of its shortcomings.

> The person in charge of this project was working on it alone

This suggests that probably the codebase started out in a neat form and became messy over time due to understaffing/bad planning/feature creep. There is only so much he/she could do as an overworked single dev having to manage a large system.

Your senior is probably very happy to have another dev helping with this. I wish I had someone helping me. Take it as an opportunity, dealing with legacy is a challenge but you can grow a lot as a developer!


> Take it as an opportunity, dealing with legacy is a challenge but you can grow a lot as a developer!

I agree, but it's fraught with risk, and could also turn into an opportunity to spend several months or even years being miserable, and thought a mediocre to poor developer because you aren't able to deliver fast enough. This becomes especially true when the original developer is there and can probably do things 10x faster (which will make you look bad).

You'll need to make sure you're having regular one on ones and keeping your team lead/manager up to date on the work you're doing. Perception is more important than anything. If you save the world but do it quietly, nobody will know, and they'll decide you aren't productive enough.


> If you save the world but do it quietly, nobody will know, and they'll decide you aren't productive enough.

I agree, I think OP should find a way to present his work as a "project" that makes sense to management... A good example, someone else in this comments thread suggested that OP could create an API around the legacy core. Creating an API is something that can be "sold" well to management.


> I've already made some comments about rewriting it and the response was basically "ok".

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...


I'd start with writing tests. That way you're more likely to notice if you break something in a distant part of the code.

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".


This is the reality, don't expect every code base is clean and well organized. I have come across the same situation for a couple of times in my career. I felt disappointed at the beginning, I wondered how high-paid engineers could write such shitty code, but quickly I found it's actually a very challenging job. To understand the code base was like playing puzzle games with debugging tools. I had invented tools with some new stuff I just learnt to trace and visualized the program, or wrote scripts to clean up the code. Eventually I became the owner of these projects, and refactor the hell out of it.


> Should I just go and basically say that this person did a bad job?

That'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.


This is how most real world code is.

You might be the exception, but when you've been working on code for a while, your code is probably like this to an outsider too.

The difference is that you have a ton of insider knowledge in your head that makes the code understandable. The person who wrote the code you now hate probably does too.

I've learned to enjoy fixing up projects like these. You can learn an enormous amount from fixing bad code, and I truly have! Unfortunately it's probably something you have to learn for yourself. Just try to have a good attitude about it OK?


Is there a test suite? Continuous integration? Test coverage being measured? Hopefully the answer to all of that is yes already, but if not, focus on that first.

Next pick a component which is particularly problematic, or which is a focus of business attention at the moment. Look at which lines/conditions are covered by the tests – are the critical parts of the code, especially the success cases, being covered? If not, write tests for them. (Covering error cases is less critical, although user errors – bad user input – are more important to cover than things like IO or network errors.) Don't get hung up on a numeric target – which bits of code are being covered is more important than a number.

Once your test suite covers the important parts of that component, start slowly refactoring it and cleaning it up. Then, once you are satisfied it is in a better state, choose another component to move on to.

Of course, that assumes you can get your management on board with you doing the above. But if they aren't keen, you could try to phrase it as "Can I spend 20% of my time improving quality, and the other 80% on new features and fixing reported bugs?"


I moved mostly to real estate after running an IT company for 15 years. You know what they say in this field about 19th century buildings, which are certainly technically inferior to newly constructed houses? „At least these buildings have been standing for a century without collapsing“. IOW, the code you are seeing is at least in production, while many other companies did things differently (perhaps in a prettier or more maintainable way) and failed.

Sorry if this isn‘t helping, but have some respect for a codebase that seems to be working. And write lots of tests before touching it.


Lindy effect: The Lindy effect is a concept that the future life expectancy of some non-perishable things like a technology or an idea is proportional to their current age, so that every additional period of survival implies a longer remaining life expectancy.


>Sorry if this isn‘t helping, but have some respect for a codebase that seems to be working. And write lots of tests before touching it.

This right here is the best advice I've seen in the thread. Lot's of people are giving good advice here.

Definately write lots of tests before touching it. As you write the tests you may even start to see a pattern or "method to the madness".


> Should I just go and basically say that this person did a bad job?

No. This has the disadvantage of possibly making you look naive. The code might actually be fine and you might not have enough experience working in unfamiliar codebases or reading other people's code. Moreover, original developers will have made choices for a particular reason, often without commenting and you might change something unintentionally if it doesn't read straightforward the first time through.

If it's mostly style issues, forget it. Unless it's highly trafficked code, the computer doesn't care if you use PascalCase or snake_case or awkward indentation, etc.

Report that you might be wrong, but the code seems overly verbose/incorrect/hard-to-read in some areas and that you are carefully refactoring, but that you are still getting familiar with the company's product and would be excited to work on a different area of the codebase as soon as this project is complete.


Consider that you might be wrong. It takes a lot of work to understand a code base. I've seen many young developers spend a short time looking at some code, fail to understand it, throw up their hands and claim it's a mess that needs to be rewritten. Put in the time to make sure you know what you are talking about before making a fuss. I've refactored a lot of other people's code in my career. If someone else wrote it then it pretty much always looks bad at first. Then again there is plenty of genuinely bad code out there.


> 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 think the only reason that you should be concerned with "this person", is to influence them (or better, the work environment) to discourage the behaviour that led to the problems you're struggling with. For that to be effective, you need to stay positive.

Remember that it's much more powerful when someone learns something themselves, compared to being told that same thing. If the code is broken/fragile, then you'll be presented with opportunities to show positive examples of how your work is improving things. Folks who know the history of the code will hopefully see what's going on.

The codebase isn't going to be magicked in to a better state through political moves, so assuming that you stay on, you'll need to come to terms with it.


It probably also depends on your experience.

You might claim it is spaghetti code because you believe in some dogma that your predecessor did not adhere to. Maybe hold back for a while and try to understand what's going on first?

I've been there as well, but after going through quite a few 'paradigms' I have to say that I prefer spaghetti to

lasagne (layers and layers and more layers of crap)

calzone (looks nice on the outside and has test cases for every line but thats because the logic is so contorted and mutable state is everywhere)

the mafia that tells me I can't use this keyword or other, how long lines can be, that I can't use a single letter variable name, etc pp


From super senior dev perspective.

You're not going to be there for the rest of your life. One year is enough. Take it easy. I went through this multiple times, there are lots of places with a shitty code. The worst thing that can happen - you'll will start blaming yourself. But if you just relax and let things go, you'll get salary, 9 to 5, job security. Don't try to fix everything in one day. File stories, fix issues one by one. Eventually you will like it, and this is the real software engineering, the real world.

Yes, there are better places, but you will never know until you get there. So if you can live with it for about a year, it's good. If not, start looking, and remember - it's not your fault, and it's very hard to avoid that. You need to work in a company for a couple of weeks to understand how things are going there. So you new job can be the same or even worse (it also can be better). But there are no any guarantees.


Go slowly and look carefully at things.

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.


One thing you can try doing is to rework sections of the codebase gradually over time as you change them. Don't touch code that is working and doesn't need to be updated, but if you are working on a feature in one corner of the codebase, see what you can do to improve that part of the codebase.

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.


Gamedev.

I joined a small company 8 months ago. The code at first felt somewhat sane, but the more I worked with it the more I understood that the whole project is a huge copypaste mom's spaghetti. I started rewriting everything I "legally"(somewhat within the scope of my tasks) could. I also kept asking the lead coder about the poor quality and poor decisions up to the point where all we did was argue. Management saw this and it turns out they were not satisfied with his performance for quite some time. He gets kicked out(after 4 years). During the 8 months I have deleted 1/3rd of the whole codebase, replacing it with much better code both quality and performance wise. I did that because I don't want to spend the next couple of years suffering from someone's bad decisions and complete lack of experience. Now I get to work on much more interesting tasks, everything gets delivered faster. Some of the stuff I wrote gets used across other teams. Only things that still keep me getting stuck in debugger are those I haven't rewritten and everyone agrees we have to rewrite them in near future.

I know I made a risky decision, but it was well worth it.


  - get assigned a task/project
  - when your changes interact with existing code, take some time to write tests
  - refactor as necessary so that you aren't making things worse
This is what you should do day to day as a software developer. Don't ask for permission to refactor, just do it (using your best judgment) as you also deliver features.


Yah i went to a big name host and found the same thing. I was fooled into joining because I met with a data scientist and an "experienced web developer" without actually looking at their code. Long story short, 2 years later it was the most frustrating experience ever and I left, hating the code the entire time. I was paid a lot and the people were cool so that made me hang around but the culture promoted hanging on to legacy code and creating scenarios where the people in charge fostered chaotic code practices by not giving a shit and always moving on to the next thing without ever cleaning up after themselves. I would recommend you to leave sooner than later as it will not get better unless you are the one that does it all or gathers some like-minded people to go forth with you but really that's too much effort IMO and you should just bail to a better scenario when you can.


> I was fooled into joining because I met with a data scientist and an "experienced web developer" without actually looking at their code.

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.)


When we first identify partners or clients, the interview is not unlike a job interview, but one where both parties are the applicant and interviewer. I cannot count the times when someone from the other side tells me about best practices in coding, CI, covering tests and version management, after which, on inspection of their codebases, they actually do none of these things. The guy at the meeting stage was a higher up tech manager/C(TI)O usually who read all the books and articles but the team is not following those same practices because of various reasons (usually intense time pressure).

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.


> Is it typical to be able to look at a company's code before signing up?

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.


> but there're no tests to check whether my adjustments are correct.

Start here. Start writing tests. Lots of tests. This is part of what makes it engineering and not just craftsmanship.

Once you have enough code coverage (this will take a long time), start rewriting small parts of the system. You will have your tests to fall back on to verify you new design is correct. For each part of the old system, once you are confident, replace with your new code.

This all needs to be communicated up front as a long term goal. It's not about a rewrite. It's about the tests and having high quality software.

Something you need to understand, they may not want or need high quality software. Quality costs money. Sometimes good enough is all they can afford right now, even though we know the tech debt will accumulate interest. A part of your pitch is to communicate how important quality is to the long term viability of the software.


Somehow a lot of people have got it into their head that all code is art, that everything is always beautiful, that it makes sense to take the time to make a great design for solving every problem, that if you aren't working with beautiful code then you aren't a good developer. That's all nonsense.

Think about other engineering disciplines - "okay" engineers can build new bridges, great engineers can cheaply fix existing bridges so they never fail again. It is the same with software. Good engineers make new programs, and whenever they see a problem, they start from scratch, wasting time and money. Great engineers massage existing code into something amazing, increasing efficiency without breaking the bank.

As a side effect, if you make something crappy more reliable, it makes the creator look bad, as they should. Otherwise, you will look bad.


Do you know why that software is a mess? Nine times out of ten, even with bad developers, it is because of changing business logic requirements, bug fixes, and dealing with corner cases. Your rewrite will, by definition, throw all that accumulated knowledge away.

What you _can_ do is separate large swaths of code into methods, even if they are only called once. This will make debugging much faster and more effective. Pretty much anytime you have a section of code that starts with a comment "// Foo the bar", you should consider isolating it into a function "fooBar(int a, char *b)" which you'll notice is even more informative that the comment, and can easily be stepped over when debugging with only the final return value confirmed to be as expected.


A manager if mine once asked the developers what spaghetti code was, and one of the developers gave my favorite answer: "code written by someone else"


This sounds normal.

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.


> Miraculously, things still got done by manually testing.

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.


Yes, I was being sarcastic with the "miraculous" comment. The right answer is definitely a middle ground...


I'd say just double down and have a job for life, https://github.com/Droogans/unmaintainable-code


Welcome to the world of professional programming. Situation normal at many many companies.

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-...


Start with writing Unit tests for basic functionality.

Do not do the cardinal sin of writing unit test for every goddamn function in the codebase like a rookie.

Start fixing code, one by one only if it can be tested by Unit tests.

Remember you need to first learn the code base and context because harping "complete mess". Maybe they did not have time, maybe timelines were more imp. That is what industry is about especially when you do not have enough Engineers. Plus this new wave of PMs who want features at any cost, doesn't make things any easy.

Good luck


You need to make a choice:

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).


Depends on the task, but if your task is amenable to it, I'd consider writing tests which basically say, at a very high level: "Is the output for a given set of inputs the same?"

Even if you're wrapping the entire application.

We're not looking for "does this code do the right thing", just "does this code do the same thing" (as the initial code).

At which point you'll at least have some more confidence that you aren't damaging the codebase due to having not understood a part of it.


this was me a few years ago, here's some stuff I learned about diving in to a codebase like this:

- don't be rude but make it clear you think the code is messy and you can clean it up. Explain it to non-programmers in terms of time needed to add new features and the much greater chance of bugs and side effects

- get a few style rules going (do A and not B, group regions/routes/modules -- with only comments at first, if needed) to apply ONLY to new code going in.

- every time you have to touch old code, bring it up to style, even if your changes don't directly address spaghettiness

- slowly (and more confidently now that you're more familiar with the code) refactor parts of the code

- get at least some sort of test structure in there

It'll take a long time but it'll mostly work and you'll probably find it very satisfying. I know I do. Be polite but firm. Believe me that everyone knows the code is a mess and if someone is willing to put in the legwork, every developer worth their salt will respect you for it. You'll have the authority you need to enforce your stylistic choices and the future structure of the code.

Remember above all else, though, that your job is to write working code, not pretty code that developers like. Your goal should be to infect it with order, not tear it out and rebuild it.


You want to do what you can to improve things, but be careful. I've been massacred in a similar position many times.

Some pitfalls you may expect:

1. You find some crazy thing that makes absolutely no sense happens in the code for some specific inputs. You remove the craziness, and someone complains that an essential feature just broke. When the essential feature is explained to you, you find that its implementation is batbarf crazy, and that although it would have been easily implemented in a more straightforward way, someone important, somewhere, is redefining inputs and re-interpreting outputs in some special cases to get a feature they needed and no one had time to implement.

2. You find some crazy thing that makes absolutely no sense happens in the code for some specific inputs. Based on your knowledge of the many pitfalls, you ask permission to fix it. The more experienced person whom you ask explains that the input conditions to trigger your bug are so rare that it makes no business sense to waste any time to change it, it's not a priority, and you will understand things better when you have more experience, and why are you wasting my time asking me if you can change it when 85% of changes in this code base introduce new bugs (because it is such a mess)?

3. You find some crazy thing that makes absolutely no sense happens in the code, not for just a small subset of inputs, but for quite typical cases. You remove the craziness, and then you learn that a 'workaround' is in place, perhaps including both other software and user procedures. Lots of people are earning a living doing the workaround, and they don't want to change.

4. You find some crazy thing that makes absolutely no sense happens in the code. You remove the craziness, and then you learn that some other system is now out of balance with yours or fails to run with the data it gets from yours. No one anywhere can figure out why, but you caused the problem.

5. You find some crazy thing that makes absolutely no sense happens in the code. You remove the craziness, and then you learn that your chief accounting officer thinks that he is a genius because he found a way to turn craziness into quarterly earnings, and he is not happy with your fix, and the auditors want to talk to you.

6. You find some crazy thing that makes absolutely no sense happens in the code. You remove the craziness, and then you learn that your CEO's brother earns a commission or royalty on the craziness, and he's not happy.

7. You correct a spelling error in a report heading, producing numerical errors in twenty-seven downstream reports.


There are multiple ways to deal with it from an engineering point-of-view but the conversation is a business one. Is the product in its current form likely to be in-use for many years to come? That is different from a product that is only used until the next large raise in maybe a year or two in which case you can maintain it until the next release.

That said, one of my favourite ways of "rewriting" is firstly to identify any parts of the system that could be separated off into a second application and start designing and building a new system in parallel.

Another relatively easy task is to spend a small amount of your working week building the "rewrite" from scratch starting at the highest level so that you can learn some of the problems that are not obvious from the current system but which the previous developers had to learn while they went along. You could build a large part of the scaffolding without struggling too much and then have the basis of something to suggest for the future.


> The person in charge of this project was working on it alone and from the outside it all looks fine and it's working.

There is a reason things are like this. One thing are "best practices", blogs of design patterns advocates, interview process, and smartass community celebrities. Another thing is real professional environment with business reasons, limited budget, and limited personnel.

> Should I just go and basically say that this person did a bad job?

It will not impress anyone. It's not a job interview where one can smear, judge, and reject the candidate for the most trivial nuance. It's your daily job and you'll have to work with these people.

Rewriting functional application during the first months of employment doesn't sound like a good idea. Use cases, edge cases, algorithms - probably took good time to figure it out. Just think about it - the last guy trying to do it ended up alone, dumping the whole codebase on someone else.


I've been there. Lots of times!

    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?
It's not easy but it's possible.

The first thing you need to do is get some people on your side, politically. If you just rewrite the code on your own, nobody is going to thank you. You might even look like a terrible employee because you're sure to break a few things, especially since there are no tests to help you.

Explain the problems with the existing codebase, and how they're preventing the team from delivering features.

One idea - give them some easy to understand graphs so they can see how much time's being wasted on the spaghetti struggles. Record your time for a week... show them a pie chart of how much time you spend actually implementing new features vs. how much time you spend fighting with the spaghetti.

Come up with a plan. Suggest devoting X% of your time to reducing technical debt and Y% of your time to delivering new features.

Come up with benchmarks to show your progress. Performance benchmarks, productivity benchmarks, anything.

    Should I just go and basically 
    say that this person did a bad job?
Be very careful about this.

You can explain the bad state of the code without implying that the previous coder did a bad job.

After all, it's normal for software to accumulate technical debt over time. This is especially common if you have a single coder that is under pressure to deliver a lot of features in a short amount of time. It doesn't necessarily mean he/she is a "bad coder"; maybe he/she was just not given time to refactor and reduce debt.

    I've already made some comments about rewriting it and the 
    response was basically "ok".
Sounds like they're aware of the technical debt, so that's a good sign maybe?

    I started "repairing" the project but there're no tests to check whether my 
    adjustments are correct.


This is, of course, step one after you get approval and before you start reducing that technical debt. =)

Good luck!!


Welcome to software engineering, this is how things are here. It is very unlikely that you will ever work on a large project that gives you a warm feeling in your belly. So learn to grow on it.

Rewriting random code as a codebase newbie is always a mistake. You will get things wrong. Mistakes can be delayed a long time before you are aware of it.

Try to understand the product. You should spend more time to understand the product features and flow instead of staring at single ugly code fragments. Understanding it will reveal some structure in the code, even if it isn't explicit. Try to engage your peers about things you don't understand. They will potentially deliver tons of detail and background information. Over time you can mentally map this high level information to the codebase.

If you understand the product better, I'd suggest to write a few (1-5) simple selenium tests that cover the core tasks of the product. This test is simply to ensure that you do not break something on the greater scale and the product is still delivering it's basic value. If you start to find confidence with you tests, you can share them with your peers. This will give everyone some instant value and potentially a lot of respect towards you.

You seem to be afraid of your colleague. You don't even know if he wrote all that code, you don't know about his stance on the codebase. Engage him, try to find out how he is working with the codebase. Try to find out his skill level and mentality. Do not judge early, don't complain. Even if he is low skilled, he could have a lot of knowledge about the product and the company.

Eventually don't expect to rewrite the whole codebase. Rewriting a large valuable project which is in production will take easily a year. One year after that someone will claim this is legacy. It won't help. It is easy to think that you can do better, but even a clean codebase can be a huge mess. Figure out how to gradually improve the project alongside features. Figure out risks and high friction tasks for devs, operations and users.


Write lots of tests first, fix the code as they break. Don't make it personal for the people who wrote it. You don't know the conditions under which they originally wrote it. All else fails, join a start up early and write a whole bunch of shitty code in an existential panic.


Should I just go and basically say that this person did a bad job?

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.


"When all you have is a hammer, everything looks like a nail"

Don't go into the trap of rewriting the code the first week, all new coders think that is the only solution on the first job. It takes time to realize why the code ended up in that state in the first place and badmouthing previous developers doesn't make you look very good. Management might very well know why the codebase looks like it does, priority is often time, not quality.

Something that looks like a bug might have become a feature that the users depend on. If you remove that bug you have created another instead risking corrupting data and/or angry customers not able to do things the way they are used to.

For all you know you are being tested for work culture and how you behave towards your collegues :-)


First is understand that ugly or hard to maintain code is not necessarily doing a bad job. The organization has a lot of different goals, and cheap maintenance is only one of them.

But from your post it sounds like your immediate problem is you don't like working on ugly code. Thrn I'd suggest trying to get transferred, rewriting jto from scratch (at most companies this would be a hard sell, and probably a money pit), or find a new job.

Complaining about the code is only going to make your workplace more hostile.

If you do decide to go for the rewrite option try to make the case from a financial aspect. Calculate the cost of maintenance, the cost of the rewrite etc...


Grass aint greener anywhere else. Prioritize learning, get better, increase the level of job in your range, get better job, loop until manager or self employed (at which point you'll have nobody to blame for your tech debt but you!)


As a system matures it is easy to see common code and abstractions. Don't blame the previous guy.

Don't be tempted to start a new shiny clean system. This almost never works (unless in demise cases) and you'll be playing catch up with features on the original system for the next 10 years.

As others have said. You have to write tests, this will cover your back and you'll understand the codebase back to front once you have - Testing/Interfaces/Simplify/Naming.

Remember, all the business logic is there. At least you don't have to do the hard part... this is a cleanup operation.


I would withhold judgement and re-assess the situation in 6-9 months. I've seen perfectly great code bases, go to hell, for good reasons ... strange stuff where we had to use goto's and sleep(),flagfiles, etc to get around bad third party multithreaded libraries. Look at any microsoft code if you want a real life example of enterprise class production code gone to hell.

middle ground would be to write blackbox style tests and make them portable to the next revision of the codebase.

don't say that last person was a bad at coding, you won't benefit from saying that.


you should ask yourself a few questions.

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?


If someone is still confused after reading these, ask yourself: Will the profit I gain from leaving this alone allow me to replace the entire module in a larger refactor later?


at the beginning of my career I read somewhere if it ain't broke don't fix it. If that is making money and you can black box it, then do not fix it, for sure you could always add some test around the black box and then move on. Try to create new software outside of that box that reflects the same ideals that you would have put into "refactoring" the "bad one".


I started working on a huge, 10yo< codebase with ~500k LOC, but this was mid/large company with >50 developers with framework version from 2013. Now it is ~300K(unused code removed, some parts moved outside+rewritten), framework is the latest version etc., this was done due to a very good leader with a vision. Nothing prevents you from becoming that person that fixed that infamous project.

It is ok to complain about code quality, as far as you are doing something to improve it. Don't ever complain about a previous engineer, they might have been just implementing the sh*t product was enforcing on them or company asked an inexperienced engineering to glue together this project. It is rarely that a developer knowingly and happily writes bad code. Try to improve it, it will move you towards becoming a senior sooner, nothing helps you in understand how to code more than how-not-to examples. :) Also +1 to do-not-refactor-immediately comments here, observe the project and company and write tests until you know the project and culture well enough, then either increase refactor speed thanks to your tests or separate the project into chunk as micro-services or utility libraries/modules.

Also run static analysis on the codebase, you will be amazed how much "foo defined but never used" cases there will be, remove them completely, have a close eye on production errors when you release them and role-back on smallest of suspicion something is wrong. invesitage, fix, release and repeat. :)


This is a familiar tale; it isn’t at all unusual to find yourself in such a situation as a new person joining a project. Despite the temptation to refactor, don’t do so without a reason —-remember that any code you change or improve can introduce subtle or not so subtle changes in its behavior and can have unanticipated negative impacts on the business. It won’t matter that you wanted to make the code better, if you touched it you will end up owning subsequent blame for problems. Refactor when needed to cleanly add new feature requests or bug fixes.

The good news is that the team and code base are both small. Bigger teams and bigger code bases (like hundreds of thousands of lines) become non-linearly harder to repair.

Likely high level goals are: being happy at your job and making your company happy to have you there. As a new person on the team, refactoring on your own is perilous and probably won’t advance you towards these goals; so, what should you do?

Often, tangled code is paired with tangled development methods. Look for places to make useful improvements in the development process. Is the code and it’s data protected by adequate backups; is it under change control; is there a release schedule with documented testing procedures; is bug reporting and tracking done; are there specification, architecture, and design documents (under change control); are there tests; are there reviews and coding standards.

During this period of time, while learning the code base, you can make yourself valuable by helping with the process. Be sensitive to company politics and don’t embarrass people that could make your life difficult at this time. If the company has a reason for or against using cloud backups, don’t try to change it, but if they don’t do backups properly suggest that you can dive in and help with it. If they are doing adequate backups, move on to look at their source control. I’d especially focus on ensuring that source code control (like git or Microsoft’s Team Version Comtrol) is being used. Help in the areas that need it the most first.

To introduce better coding you could ask the person in charge to “help you” by reviewing your code. This will allow you to talk about your ideas without directly challenging their existing code. Act humble, but more than that be humble about receiving suggestions for your own code. This will foster an environment where better code will be produced.

I was working on a big OS kernel project once, and a new contract programmer joined the team I was leading. He spent the first few weeks getting a better kernel debugger running, while keeping me up to date on his progress. Other developers started going to him to get help with their debugging and soon, he became that essential guy that everyone respected and needed. At that point, he was able to start suggesting changes to parts of the kernel that weren’t even his. Dispite the huge code base and the politics of numerous departments and hundreds of programmers, this person made many important improvements throughout the kernel over the next couple of years and was viewed as one of the most valuable developers in the organization.


There's a lot of good advice here, but I'll add one thing. Learn the codebase really really well. Figure out where the seriously bad problems are. Figure out why the codebase is this bad, ideally from the original developers (not in an accusatory fashion, try to frame it as learning from them). Figure out the context of the bad code, otherwise you might end up repeating it. Write up notes and sketches and plans of the bad code.

Then, isolate a section that is practically reeking. Maybe it's even so bad that it's leaked into the user experience. It has to be a super small part of the codebase though. Plan out the refactor, making sure to make it backwards compatible (that means no regressions, no changes to internal interfaces, etc.). Start the refactor, then when you reach about the 70% mark (which will be more like the 30% mark considering regressions and stuff), try to pitch the refactor. But don't pitch it as a refactor. Figure out any problem that the user or manager is having, whether it's slowness, crashing, etc. and then slide in your refactor as the solution. Like hiding spinach in brownies. If there aren't any problems, well, then maybe the codebase isn't that bad.

And remember, 50% good code and 100% done is better than 100% good code and 50% finished when it comes to refactors.


Maintaining (fixing and updating) legacy code and be every bit as challenging and rewarding as building from scratch. You can learn a lot of skills that will also help you when you are building new code. Code reading and the art and discipline of minimal change while improving clarity are always useful if applied to legacy code or new code.

Look into various books and articles on refactoring. Don't change things open-loop, write tests if you can, manually run and verify too.


This is why I'm most interested in two things when interviewing for a job. 1) Are the devs happy/How long have they been here 2) can I see code?

I have been there and the sad truth is that for most codebases I'm going to argue that bringing the whole thing up to a nice status is going to cost too much if it's already in production. Unless it's going to be running for many many years, it's probably not worth the investment. And if it's not worth the investment, then there will be no investment. That may be the correct management decision - but it's a huge red flag when choosing a workplace.

If the code isn't in production already, then it's another matter. Then you can make it worse before it gets better, and it may be a productivity boost even if the cleanup takes several years.

The thing to look out for if you can't clean up the whole thing, is any kind of modularization. Can you suggest that some subsystem is either rewritten or refactored? Make a business case such as "we have so many bug reports in subsystem X" or "We could easily make Y processing 10x faster". Even without tests, it's sometimes possible to implement a new subsystem and use a previous subsystem as the test - your new subsystem works if it responds the same as the old one.

If you are not allowed to take an architectural or greenfield approach (i.e. you can neither rewrite or do large-scale refactorings of at least parts of the code) and you are simply asked to provide small focused (business-driven) changes to the code, and management shows no clear dedication to large scale refactoring or rewriting, then I'd reconsider working there.


Sounds familiar. I've worked on a similar code base, old code written during the startup years that is messy and have lots of subtle bugs and is a complete dependency mess.

When I joined I was told by one of the (post-startup) devs he had a rule: whenever he had to fix a bug or add a feature to a piece of code, he'd try to clean up the method or class he had to work on to implement the change. Bigger changes then allowed for bigger cleanups.

We also had the challenge of testing. So we decided to accept the situation, that we have a fragile project and that our changes will lead to bugs, and instead wrote an infrastructure that made it easy for our users to launch older versions of our application. For the most part this meant blocking bugs were not such a PITA for users, if we broke version 42 they could easily launch version 41 and use that until version 43 with a fix came along.

As for saying the lead guy did a bad job, I think you're making the mistake of looking at the code as the most important thing. It really isn't. The application and its functionality is the most important thing. Functionality is what makes the application useful to users. An application with no users is useless.


Well, first get everything under source control, if it isn't already. Get the build process organized so anyone can do it.

Start adding comments without changing code. Any time you touch anything, comment that area as you figure it out.

If you find unused code, put something like #ifdef OBSOLETE around it, rather than removing it immediately, and stick in "assert(false)" where something is unreachable.


Before making changes, ask for the up to date specification. Then design tests to verify that the code implements the spec. Get the product owner or your boss if more appropriate to confirm in writing that the tests will be sufficient to prove that the code is correct.

Then implement and run the tests. The code will fail some of the tests, so make sure that those tests really do correspond to the spec and then present the remaining failures to product owner/your boss. Maybe the spec will change, and then the tests will change and the code will pass. If there are still failures then get written instruction to fix the code.

Eventually you will have code that passes all the tests.

Now you are in control. You know what the code should do and that it does it. And best of all, you can make changes to the code and know if the code is still correct.

Additionally, you might find yourself looking at the awful spaghetti in a different way. After all, the spaghetti does exactly what it's supposed to. Maybe you can make it do it better and be more maintainable etc, but as far as anyone else cares, it'd better taste exactly the same or else you'll have screwed it up.


Two thoughts came to mind when reading this.

First, remember trust is normally earned, rather than assumed. There is a careful balancing act between getting to know the code base, the culture and the team vs being proactive from day 1. If they're completely lacking tests, write tests for the sections of code you're working on; the sections you fully understand. There's a few books on the subject, but they all boil down to don't try and tackle everything at once. Build an understanding and ensure new code is well tested.

Furthermore, make sure you understand the background. Is this a prototype that has been deployed to production? Is your colleague a "starter"; great at shipping new ideas, but stability and maintenance are not their strong points? Do they simply know no better? Why are things the way they are? The solution varies a lot.

Lastly, age, experience and knowledge are often confused in this industry; try to keep them separate. I manage a team of 6 and I'm the second youngest member (late twenties), but the most experienced in my product. It takes a mix of everything to make a good team.


Like it or not, it is your responsibility to inform management if/when there’s a problem. It is their responsibility to decide what to do about it. Be transparent about the impact that the current codebase is having on your estimates, bugs in your code, and job satisfaction. Analyze the situation and offer solutions. If you have provided management with accurate information, you will have no reason to feel bad about the outcomes, and there should be no surprises for anyone.

I understand that you do not wish to offend your coworker who wrote poor code. That’s good, but it is also your responsibility to find a non-offensive way to communicate when their work is not ideal. I recommend using “code-first” language when talking about the code. For example, do not say “what you’re doing in this function is wrong”, say “this function is incorrect” (basically just never say “you” when discussing the code.) If they get defensive, remind them that your goal is code quality, and that you are not trying to be offensive. Be precise, informative, and most importantly: humble.


is improving old code really what you want your career to become? this is a selfish question, but you need to put your own needs before that of the company. you are not a charity. they are selfish. be selfish.


Look at the code coverage. Not just the line percentage, but look at the more complex pieces. Are they covered or not?

Don't refactor it right away. I'll take your word that the code is shit (frequently the case), but you don't know the idiosyncrasies well enough to know what to do with it.

After you've worked with the code a while, you'll be able to attempt a large refactor. Until then, gather low hanging fruit. Clean up confusing variable names. Add comments where needed. Write your new code as clearly and cleanly as possible. Use a tool like sonar to inspect your new code so that you don't introduce new problems.

Most importantly, ADD UNIT TESTS. That will make sure that your new code works with the old and it will allow you to attempt a larger refactor while being sure that you aren't breaking anything.

If necessary, you may need to add some integration tests as well.

I've been in your shoes. Bad code is no fun, but don't let it get you down. Focus on the happy parts of your job (team, culture, whatever). Good luck!


Definitely seen this first hand as well, as well as seen change happen!

While I agree you should not attempt significant rewrites right away, and spend time to develop your knowledge of system... don't just sit back and wait. That won't help solve the problem.

1. Be up front about your opinions and rally the group around the long-term goal to improve code quality. Do it in a positive manner where it's clear that you want to make an impact and help make changes for the better! (not just complaints) 2. Explain your reasoning clearly (testing, code quality, engineer happiness, etc) and ask for input / get buy-in. Open up the discussion. 3. Bring this feedback up consistently as an important part of your experience at this company (in 1-1's, and other team meetings) Good ways to do this are when discussing OKRs, retrospectives, etc.

Believe that YOU have the ability to spark a change and develop real pride amongst the team! If you don't feel this is possible, this is not the right place for you.


Real codebases that provide value to the end users are often old, big and ugly. That's one of the reasons people pour hours of their free time to github and such to produce something that is actually beautiful.

"relatively big spaghetti (tens of thousands of LOC)."

That's not very big.

"Should I just go and basically say that this person did a bad job? "

Definetly not. Just writing ugly code is not malfeasance. If the code actually works as expected once compiled then you have very little basis for this accusation.

Functionally complete and maintainable are two different things, and how much the second is cared about varies A LOT.

"The person in charge of this project was working on it alone and from the outside it all looks fine and it's working."

First, and the second metric of code in business is that a) does it compile b) does it work.

All other considerations are secondary.

"Maintainability" has no absolute metric. Organizations learn slowly about this, and see how it affects their internal capability to deliver, but there is no way you can directly point out based on what metric the code is faulty.

The guy before you did not do a bad job if it actually works.

The time to point fingers would be if there is no source code to be found, you found out it's some repurposed third party codebase with an incompatible license related to the business use of the code, you found out he had sneaked in a virus to the installer and so on.

'I've already made some comments about rewriting it and the response was basically "ok".'

Sounds like an ok way to move forward. Make sure you understand the business value of the codebase as well.


> Should I just go and basically say that this person did a bad job?

God, no. If the code works and scales, he did not do a bad job.

Start to apply your new standards for your new code you write on it - make tests for it too. You might need to add a few tests for the most important parts of the old code to make sure nothing breaks but I would leave the old code be as much as possible.


I agree that it can be frustrating, but you don't know the constraints that the previous programmers were under, or if it's part of the "prototype" phase.

The best thing to do is to gradually rewrite it by always leaving the code in a better state than it was before. For example, you're tasked with a non-trivial adjustment to an API endpoint--instead of adding to the mess, extract the endpoint out into it's own file and rewrite/clean it up. Write tests beforehand if that makes you more comfortable.

Using this technique, my current team has been able to slowly rewrite our codebase from spaghetti JS (read: thousand line JS files) into a more structured TypeScript project. It has a long way to go, but doing an incremental rewrite works wonders.

And like another poster said, being able to read legacy code is a good skill to have. After all, you'll need to understand it before rewriting it.


I think it is a good trait that you care enough about your work to feel this much frustration. Change though, is often hard to achieve because people tend to be stubborn. To provide more useful feedback I think you should mention what is expected from you at work. Are you supposed to be working on new features in this project or just maintainence?

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.


If the original developer is still around, have some informal chat here and there on why the worst part are the way they are.

Frame it correctly: "I don't understand why these classes share this data or code, do you remember why?" is miles better than "Explain me this mess please it's making me crazy"

If the original dev is still around focus on the task that come from business so he's free to focus on fixing the structure, he is, after all, the local expert on it.

In general a rewrite is not something you'd do without having a clear understanding of the requirements. What you can do instead if you don't have the dev support is to fix thing as you go, removing cross dependencies and encapsulating behaviors. You don't have to fix code only to hide the ugliness behind a clear structure and as small as possible interfaces, order will eventually emerge.


What you describe is called technical debt. https://en.wikipedia.org/wiki/Technical_debt

A strategy of unethical developers is to make themselves relevant by producing a lot of features while drowning the project in technical debt.

This often happens when:

a) stakeholders are not technical.

b) their strategy is to have an early exit (like an acquisition) before the problems caused by the technical debt materialize.

c) they can compensate for the technical debt hiring an army of thousands of developers.

I would say it is a management problem. And like all management problems, unless you can fix that, your best option would be to spend your peak productive years in a company that cares.

If you want to take issues to management, tread very lightly. First of all, your challenge will be to present the problem while not seeming depressed, irritated or pessimistic, or heavily criticizing others. Try to sell the idea using a positive twist, like increased productivity, robustness, etc. If that doesn't work, just start interviewing.

---

If you decide to stay, just start creating tickets from all the technical debt. Make sure to include details like commit links, and objective information about how it is a problem. Then:

- link bug tickets to tech debt tickets.

- link incident tickets to tech debt tickets.

- if a task requires a tech debt workaround, create a separate ticket for the workaround and link it to the tech debt ticket.

This should create some sense of visibility around the tech debt. It will likely won't work anyways:

- Some people feel smarter and empowered when they deprioritize tasks.

- The only ability of uncreative managers is to say no. Saying no makes them feel useful.


Assuming your project has an issue tracking system, start adding issues - bugs, technical tasks, improvements, etc. for things you find and discover. If there are no unit tests, add an issue to create unit tests. If there are missing unit tests, add a technical task to create the missing unit tests or expand coverage.

If your project doesn't have an issue tracking system, then recommend something simple that fits into the tech you have, i.e. GitHub or whatever.

Use the issue tracking to create and document the shortcomings and technical problems you're seeing so that the more senior and leads of the projects have documented visibility into what you are seeing with the code and the application.

For example create a technical task: "Add unit test coverage for user sign up" or for whatever gaps you're seeing in testing.

In my projects I always make sure that we have a category available for "technical tasks" and/or "system stories" to document anything that is engineering/code/refactoring/devops related vs. user facing functionality.

If these channels don't exist, then you need to recommend that they are established so you can document and track specific issues you're seeing.

Also, since you've recently joined, any team that brings on new members should be open to what you're seeing as someone new with a fresh perspective and be open to learning from you as much as what you need to learn coming into a new project. Hopefully, your team has a culture that is open to new ideas and if so, don't be shy on bringing up technical issues in standups, chat discussions, PR comments, etc.

Just be proactive in documenting your specific issues, ideas and what you're seeing, ideally through the established issue tracking system. Be prepared to offer, document and communicate solutions to the problems and gaps you're seeing.

Good luck with the new project and I hope it works out for you and your team.


Whenever I'm confronted with a situation like this - my first go-to response is to start documenting as much as I can. If the code is properly commented you may be able to use a tool like Doxygen to spit out some HTML documentation, which at least gives you a starting point to talk to people about the codebase.

Generally speaking:

1) Stick to bug-fixing for a good few weeks / months. This is invaluable for figuring out the mess and understanding why it is the way it is.

2) Ask questions - don't be afraid of asking basic questions if you think something looks silly. More often than not there's a reason.

3) Improve the code in small chunks, but be mindful that what you consider an improvement may not be.

4) Understand that working as a team means you will need to accommodate varying levels of skill.

5) Don't blame individuals for bad code. Leave that to your superiors.


Bad code is something like a constant factor, you cannot avoid them even in somewhere like Google or Facebook. The variable here is if people are willing to accept improvements. Unfortunately, people usually tend to stop you from bringing major updates to the existing code base because this "could" hinder their immediate works. In this perspective, you're very lucky. At least, you don't need to fight your way to make a simple improvement because someone else just want to keep it as is.

But keep this in your mind; if you make their life harder by your rewriting, then they will very likely make yours painful later on. Bring gradual, safe but immediate improvements and gain other's trust first. Then thing will get much easier next time, even someone else could do the job for you!


Even if I trust you about the code being really bad, I'd still advise you to read this: https://www.joelonsoftware.com/2000/04/06/things-you-should-...

Joel makes it pretty clear that all codebases go through some kind of "maturing" where the quality is lacking, usually. It kind of made me change my opinion and approach to old codebases. Still, I think /u/freetime has the correct idea in not jumping into refactoring the first weeks of your job. Makes you look like a jackass (I made the same mistake at my current job). It might hurt, but I'd also advise to wait with improvements.


“If it looks stupid and it works it ain’t stupid”. On a more serious note. Always share your findings. First with peers, second with managers. Either you’ll learn why it works as it is or you show a risk and opportunity to improve. Nothing to lose in either way.


Get the most important things right first.

Make sure the full project is split into some reasonable modules with well-defined dependencies. The internal quality of a given module is less important as long at what it does and what it depends on is clear.

It can be easier and more powerful to throw out / replace than refactor / rewrite. Modularization is what enables this.

Make sure that version control is properly used (everything including configuration is under vcs, what is in production can be easily produced, commits are done in small meaningful units), builds are reproducable. These things are more important than detailed unit testing, documentation and test coverage.

Lastly. Get used to it. Most larger projects have tons of bad code.


Sadly, if the code could be split into modules without very high risk or massive work - then it's already in pretty good shape. The most obvious sign that a codebase is bad is that it cannot be modularized at even a few points without breaking compleytely, and doesn't have the test suite to guide that process.


Upon reading that, my idea is that that person had too high a workload to do it all the “right” way, was learning on the job, or both. They might realize it all could be more clean and proper, but not have the ability to make that investment in fixing their technical debt.

On my own code, every time I have gone in and thought I was going to fix it by refactoring or rewrites, I slowly and sometimes tragically re-learn all of the edge cases I was writing around and frustrating things I discovered when I was writing it in the first place. There may be non-obvious problems regarding other peoples’ APIs, for instance, that necessitate methods which appear to be poor practice from the outside.


I am actually in the exact same position, in fact if the person in charge of the code base i am working on checks this thread will most likely think i am the author, and that would get me into trouble, that's how similar our situations are xD.

The only difference in my case is that i have to add more functionality to the project, i started by trying to refactor and clean the code, after a while i realized that this is almost impossible and has no visible benefits from the higher managers's point of view. So i adapted my self to write spaghetti code like the one i am facing, just to meet the deadline, and i am planning to get the hell out from here next week (resign).


If 1) there's a solid engineering case to be made that this code is bad, 2) the code does something that's important, and 3) the company doesn't want to fix it, then you can be confident that the company also isn't interested in investing in the things that you as an employee would want them to be. I would expect work in general to be unenriching, raises to be smaller than at other places, and other benefits to be not as good. This could be an early sign that spending time with this employer is bad for your professional growth.

Edit: definitely kick the tires a bit by seeing if this situation is typical for the company, or if it's an outlier.


If there is no tests to back you up, that's where I'd start.

I work in a similar company in the past, and having tests / a build server is step 1 to empower other to refactor more. The lack of tests is usually what brings people to paralysis / creep / huge tech debt ime.

And don't forget that beautiful code is not actually necessary for business. What is needed is delivering value to your users in a consistent manner. Well structured code will be needed if the project is here to last in the long run, but it is not always the case.

As far as I can read, you're doing great. Keep it going and other are likely to look up on you in the future, and you'll grow a lot.


Stay or go. That is your first decision.

If you stay: After 3 months you should have a good feel for the project. After 6 months you should be able to regularly make small contributions.

After 9 months: Spend 10% of your time documenting the problems you see and the changes that might solve those problems. Spend 10% of your time planning another work project that is unrelated to the main project. Spend 20% of your time honing a skill you want to learn. Spend the remaining 60% doing the work you were hired for.

After a year, if you feel the same way, discuss with mgmt + senior the problems you’ve been documenting and if the changes you suggest are something that should be implemented.

Iterate.


I have a similar problem, but they are actively making new messy code.

When basic things are not being done right, it feels almost demeaning to try to start a conversation with someone that the variable name "stuff" might not be descriptive.


I was in the same situation. It's bad working with poor code, but it's ten times worse having them pile on shitty code every day. And oh my, was it bad. There was no code reuse, if they needed the same sql query in another place, it would just be copied over. Or if there were two slightly different queries based on some parameter, there would be an if/else statement and two separate queries - often with embedded variables (hello sql injection). Routing was one big switch/case statement. Commit messages would often be non-descriptive, like “chk”. Frontend was a special hell, jQuery spaghetti code, mostly single page, with multiple levels of nested ajax callbacks. I tried to be patient and teach them, but the bad practices continued. I even managed to rewrite parts of the backend with a framework and an ORM, but the original devs kept writing code their way. At some point I just needed to protect my sanity and left.


I'd suggest a two-stage process:

(0) Learn how they do things there, and why, and gain the respect of your coworkers by getting really good at it. You won't find you can make much progress along any axis without people perceiving you as already being successful.

(1) Write all the unit tests you possibly can for as much of the codebase as you possibly can, and try and get the rest of the team onboard with a solid testing philosophy. Extoll designing for testing and testability.

(2) Use these to confidently move through the codebase and replace portions as necessary.

This is the 'replacing the engine while the car is on the freeway' approach.


The hard truth is, you're best leaving it alone unless you're willing to take the flak for introducing bugs into "working" code. It's just one of the unfortunate realities of software development.


This thread is full of valuable advice, and you should probably bookmark it and read all of it again at times. But if I had to pick a single advice as the most important one: lose the term "rewrite". It's way to easy to get a deadline or time budget attached, and that's a very bad place to be in, especially if people start to think the rewrite stalls regular development. Improving an existing codebase is not a task, it's a process. You make it part of your everyday routine, deliver features on the way, and your budget for improvement is virtually unconstrained.


I'm in a similar situation (small company, less than 50 employees, 4 engineers on the core system), but I feel fairly comfortable tackling it as I've done similar work in the past - basically rewriting from within. For me, key is dont throw it all away and try and start from scratch. Do the rewrite from within. Identify problem points and tackle them one at a time. Try not to get too ambitious. Try and keep the scope small. Doesnt always work, though. Thought I started a small scoped change a few months ago, ended up with a +600 file commit, the code was that tightly coupled.


Install phabricator somewhere if you are allowed to. Use it to add comments/reviews to the code, for now for yourself but later maybe to share with the team. Create a branch to add your code changes toto, except for the tests that you are adding to the main branch.

Remeber, code is the embodiment of business rules. Often business are not aware of the rules until after thing work incorrectly. Non-beautiful odd can be the result of programmers dealing with the messy realities of trying to satisfy various stakeholders who are at odds with each other and can't articulate what they need.


Lots of thoughts. This is a very common situation.

If you're much younger than this other developer both professionally and biologically, then just fall in line, to be frank. Do your best. Try to help them out. Always be learning. Eventually maybe they'll move on, or you'll find a better opportunity. But don't under any circumstances badmouth anyone, or try to undermine anyone, or get into an it's-me-or-them situation, these things will only set you back.

I think if you are in the position of asking your manager for permission to refactor or rewrite or whatever, the problem has been framed incorrectly. Managers are for helping people work together, and helping them understand what is important to the rest of the company, and helping the rest of the company understand what you are doing, and helping you understand how to succeed at the company, and... many other things along those lines. They're not for telling you what buttons you're allowed to push on your computer.

Your high level goal should not be to own the code, but rather the problem that the code solves. Then you are in charge of deciding whether to refactor the existing code, or make a fresh start, or something in between. But you have to make the right decision. As a professional, you take responsibility for doing the right thing given the context in which you operate -- the state of the business and the people working around you and of course the existing systems in place. As a professional, you are trustworthy and trusted. You write code when it is appropriate, solving the most important problem at hand in an efficient and maintainable fashion. You account for both short term and long term priorities when making decisions. You communicate thoroughly. You don't make big mistakes, and you own your small mistakes and fix them as soon as possible.

In your career decisions, you should think strategically and follow a path that leads where you want to go. Will this role help you become a mature professional? Do you have mentors to learn from? Will you have a balance of maintenance and greenfield development? Do you see people working there who have healthy levels of autonomy/mastery/purpose?

So overall, just keep your chin up and do your best.

But damn how do they not have tests.


Start deleting the shit code. On Fridays I went on a rampage and ended up deleting 50k lines of code. I spent some downtime and just randomly updated some really old dependcies. Then find particular areas that can be isolated but keep it small, and include reliable testing data.

Don't be afraid to pragmatically say "this code is dogshit". But don't ever expect a full or even a large rewrite. But you can move the bar forward one piece at a time.

If you have the flexibility, just go do it. As the phrase goes, better to ask for forgiveness than permission.


1. Give yourself a few months to learn the existing codebase

2. Document the existing codebase

3. Start writing unit test and/or integration test for the existing codebase

4. Adopt a mindset of ZERO tolerance for defects making it to production

5. Begin at the lowest layer of the codebase (usually data access) and execute refactorings there to either encapsulate areas of potential change or business domains

6. Once the data access has been refactored to "good enough" work on the next layer up and continue this process to the top, this should enable you to have an architecture that is closed and decoupled.


In that position I would highlight the lack of tests as a problem. This can be done “nicely” if required. “I know there was probably time pressure but without tests only the super human awesome XYZ can safely make changes and littlenold me really needs some help & protection” or done in a not so nice way “well the first thing is to finish what’s there by surrounding with tests, I’m surprised there are zero allready, it’s kind of in professional”

Then concentrate on slowly but surely cover & refactor to support improvements

Much safer than a rewrite IMHO


>Should I just go and basically say that this person did a bad job?

You wouldn't be correct in saying that. That person delivered a project that works. In the beginning of a project, that's all that matters.

>I'm so frustrated and depressed by it.

Should you feel like that? It sounds like you're working to make things better.

This Simon Sinek interview about millennials in the workplace might resonate with your situation: https://www.youtube.com/watch?v=hER0Qp6QJNU


I'm increasingly surprised at how much code I see without comments. Sometimes hundreds of lines without a single comment. And the author seems to act as if it is a badge of honor. "My code is self documenting" or "I know how it works -- it's obvious to me".

Then the author leaves, and management assigns someone else to take over the code, and he/she flails for months. It seems so inefficient. Management prioritizes shipping features over long-term maintainability and tests.

Are comments considered "old school"?


Read this about rewriting: https://www.joelonsoftware.com/2000/04/06/things-you-should-...

It's possible to rewrite but you incur risk when you do. Add tests and then refactor. Measure twice, cut once.

Or move. It's possible the person hates what they wrote and just escaped. But do the best job you can while you're looking for a better role. And focus on business value.


Welcome to the real world. I had the same thoughts when I first joined.

Learn to enjoy the feeling of thriving in a mess. Don't let yourself become fragile by it. Try to slowly turn that frustration into appreciation that you're growing as an engineer. Being an efficient, competent and likable worker even around ambiguity, spaghetti code, pressure and less-than-optimal communication will make you a rich and happy engineer.

The industry is much more than just hard skills. Build trust and improve as you go.


"Being an efficient, competent and likable worker even around ambiguity, spaghetti code, pressure and less-than-optimal communication" most underrated engineering skill IMO - if only there was an interview question that could tell me that about candidates


Make code hygiene your mantra. Piece at a time, make existing code better and make your code obey all the solid clean code principles you care about. If you want to TDD, make artificial seems on everything new and do TDD on the new stuff and start putting tests around areas at the smallest levels you can and start refactoring slowly in areas that you are touching. Make it a process that you always follow to have good hygiene. You'll eventually end up with clean code.


> 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.

Even the worst projects tend to have logging. You can hook into that, and whilst it's not great, you can test that correct log messages are made.

The TDD book, does in fact step you through examples with no tests and show you how to hook them in.

It isn't quite as hopeless as you think it is. In fact this is the kind of system I quite enjoy working on.


Working with bad legacy code is a big part of being a programmer. Legacy is the single most important area I ask about in interviews. As a medium level software developer you should be completely comfortable working with legacy spaghetti and I'm surprised that you seemed shocked. The reaction you have is typical for junior programmers who haven't yet realised that this is the reality in professional programming. There's nothing unique about your case.


Welcome to the industry. If I had a nickel for every code base I came across that gave me nightmares, I'd be able to retire.

Depending on the company, I would just keep my mouth shut. Specifically in corporate environments, where ass-kissing gets you promoted faster than skill level. I've experience this time and time again. I'll go into a company, realize the code base is trash, I'll say something about, and then not couple months later I'll be let go.


You have 2 options.

1. You rewrite the code. You said you would hate that, so not an option 2. You develop on top of the spaghetti code. You said you would hate that.

So, no matter how, you would hate working at this job.

That's why the only option left for you is really telling your boss how things are and that you can't see yourself working on this project.

You'll probably receive an offer with a 50% salary increase, so you can take that, but you should have a new job lined up already at another companyy.


You don’t need anybody’s permission to gradually add tests and improve the code base little by little. Don’t ask for permission, just make it a little better every day.


Read "The Clean Coder." Robert Martin addresses this very issue directly. He mentions some modem code that was hideous, but worked very effectively. Knowing that code doesn't last very long before having to be replaced, he realized that it wasn't worth the confusion. Look just a bit longer term than your first year or two: will this code end up being overcome by events within that timeframe? If so, don't mess with it.


Worse than maintaining a bad codebase written by one developer is to maintain a codebase that lots of devs had their hands on and had lots of freedom to do whatever the heck they wanted. Even worse than that is if devs attempted to clean the project and failed, got scared or frustrated and left the company. Now you have multiple messes made by multiple people, it becomes a mega nightmare that nobody sane is willing to work on.


Popularize the value of tests. Test, refactor, repeat. 2 years of legacy spaghetti isn't really that bad. I work for a company whose codebase is at least 14 years old in places, and there have only been serious testing efforts for 2-3 years.

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!


"On your first day at the new job, squash every commit from the repo into a single commit with message "Legacy code" and force-push to master." Source https://twitter.com/codeinthehole/status/1029682224713617408


Also, https://medium.com/feature-creep/the-software-engineer-s-gui...


You should read this famous post by Joel Spolsky: https://www.joelonsoftware.com/2000/04/06/things-you-should-...

I'm sure there's some counterexamples, where a rewrite was the right thing to do. But be wary.


Been through this in one of the largest car dealer companies in the world. Get them to buy third party components and slowly replace existing ones. Implement new features inside new, bugless, tested environments. Try moving sql to procedures and table functions. Add missing features like udp syslog logging, CI/CD, etc. That will increase developer happiness.


You should read The first 3 chapters of Martin Folwer’s Book on Refactoring. Also, if you have a product owner work with that person so they understand the situation. If you have a lead the week their guidance. Lastly, learn from the last person’s mistake and document the code so you can move to the next project. Likely that part of the code is a mess too...


Protect yourself: put everything in writing, and decline to make any change that you are not confident that you understand. It is not your responsibility to fix this situation, and don't let yourself think so.

If you change the code, you become implicitly part-responsible for it. And if you change complex code that does not have tests, you will break something.


Look through the code base and make notes on how you would approach the rewrite. Break those notes down into tiny pieces. Break those into tiny conversations and get leader support on smaller things with recommendations for improvement and get those things into user stories/tasks they approve of. Triple all estimates because you're new


> "The job basically turned into something I hate - I'm just rewriting the code..."

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.


1. take the initiative, fix what you can, quickly and quietly. 2. practice emotional detachment 3. think of the work as manual labour 4. re/read people-ware 5. this isn't personal 6. if you're good at it, see 1. 7. if you're not, look for the next job, where you can write code from scratch 8. what is the problem, really


> 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.

If you were to apply TDD to this, you would begin by writing a test verifying existing code, then you will piece-by-piece rewrite it and use your tests to make sure you didn't break stuff.


This. Don't change the code until you have reasonable test coverage. Only changes to enable testing should be allowed.


Sounds a lot like the company I work for. Don't worry incompetent idiots will be promoted to team leads and managers.


>I started "repairing" the project but there're no tests to check whether my adjustments are correct.

Yea, so you need to write some tests before you worry about that.

Fixing tech debt on a large mission critical code base requires years of work. If you stay a year often the best you can hope for is to leave them behind a decent set of tests.


> Should I just go and basically say that this person did a bad job?

You have no idea of the conditions and constraints this person was working under. You don't know if you would be able to do a better job given the same conditions (spec, schedule etc.). It is possible they did a bad job. It is also possible the did a very good job.


Before going to management to talk about someone, I would talk with that person. Maybe he/she hates the code as much as you do. Sometimes, shitty code just happens. Avoiding it is part of the job, but not always a possibility.

And if you have the option to rewrite it and you think you can do a good job, go for it and good luck!


Lot's of good advice here, but another thing to keep in mind is that dealing with legacy code can be fun sometimes. It's quite a challenge to deal with and the victories you achieve, even small ones, feel pretty good.

Also meditation is good. Sometimes I get so upset at legacy code that I have to stop and meditate.


My advice is you should refactor in multiple steps: - Refactor based on purely functions (argument -> return value), no changing any behaviours here, as well as adding some disciplines to old code base. - Refactor based on some good practices to enforce guidelines and architectures for future features.


I’d probably start by writing unit/integration tests. Once these are in place, you can easily start „fixing” the code. The thing about tests is, especially integration tests, it is relatively easy to explain that you write them to better understand ins and outs, as well as business requirements.


Small improvements on every feature you write. Just keep iterating, sometimes things look bad and you want to delete useless lines. But those useless lines may be there for edgecases that you don't know about.

Enhance, clean, iterate. Inspire the other devs to write cleaner code by leading by example.


medium level = 2 yrs experience? with degree?

i have news for you, it’s like this at all companies.

you can’t fix it. it grew that way from the people there. you need to outnumber them 2:1 to make headway.

grin and bear it. fix what you can along the way. trust me, the next guy will consider your code garbage. no matter how good it is!


At some point, you will realise that your job is not to create an idol to the software-engineering gods but rather to have a working product.

Does this product work? if so, its doing its job. It may not be pretty, it may not be maintainable, but these concerns are orthogonal to its value.


How much do you like this job? If you're the only one motivated to do this cleanup and rewrite then there's a bad culture at that company, and I'd guess very little mentorship and room for you to grow. I'd look for another job soon.


My quick advice would be to:

1) As tempting as it is, DO NOT refactor the code right now. Do the minimal amount of work needed to complete the task.

2) Start etching out some time with your manager/tech-lead to start writing unit tests for the code.

3) Gradually refactor code that is unit tested.


Don't wait it out. If you don't like your job, do something about it. Either change the work, or change jobs.

To change the work I suggest communicating with the people around you:

Talk to the previous developer, explain him/her what anti patterns are, why testing is important and perhaps venture into things like solid design or design patterns. Is he/she willing to listen/learn? Then he/she might also assist you in communication towards management. If not, then I personally wouldn't go to too much length to save his/her ass.

Communicate to management and talk their language as much as needed. Talk about what the cost of maintenance is and why finding bugs in production is so much more costly then while writing unit tests. Talk about the cost of change in this versus a well designed and tested system.

Then decide with management and hopefully the other dev what to do.


Don't complain, don't rewrite yet. Instead start slowly adding tests so that new functionality can be added without fear. Eventually you will cover with tests almost everything. Now, start rewriting it by small bits.


I'm going to embarrass my friend and plug his leanpub book, Scary Large Code Surgery with Pivots, because it was so similar to your story, except he wasn't new in his career.

Step 1 of rewriting a code base that doesn't have any tests: commit to not rewriting anything until you've covered it with tests and understand all the moving parts well enough to be sure you know what you've broken.

Chorn asked me to make sure that nobody pays for this book, so I (...won't provide the link... edit: free link below), it's on Leanpub. The idea of a "pivot" is to invent a mechanism that lets you treat each line of code the same. You need to have a process that you can use to catalog, understand, and test everything objectively and without getting hung up. If you removed any arbitrary line of code, or changed it to call a different method, that should cause a test to fail. The most important part (and it's been a while since I read the book, but...) commit that you are not going to fix anything until you have tested everything.

It will be grueling. The net effect of this hell should be that you will never question, or allow anyone to question, the value of automated testing at build-time ever again. The tests are what allow you to refactor with confidence.

If the management is on board with this strategy, then you have already won half the battle.

They may already know they did a bad thing, but your outlook on a piece of code is different before it has been made successful and people are using it. "What am I gonna need that test for? That shit is a waste of my time, I don't even know if this is gonna work in the first place." That might not even have been wrong at the time. There are plenty of ways that people can convince themselves not to write tests. Some are external pressures. If you build a great piece of code and are late to market, beat there by your competitors, that could be an even more serious problem. If the code is poorly designed, but it is working and people are using it, now you have a different problem.

The main idea of the book is that when your code has become successful and is in use by hundreds of people, the game changes. You can't responsibly make major changes to code you don't understand fully. Take the time, that's the job! And good luck.

Edit: I found the coupon link, coupon price $0.00

https://leanpub.com/scary-large-code-surgery/c/ohai


Welcome to the real world.


10's of thousands of lines is not 'big' by most standards. Just plug away at it incrementally increasing your understanding of the codebase and making it better as you go.


Identify what the thing does and if it does them poorly replace the thing. If you're thing does too many things break it into smaller pieces that do things individually. Repeat.


> I started "repairing" the project but there're no tests to check whether my adjustments are correct.

Write the tests first, that is your first job. Make changes after that.


One advice concerning adding the tests for legacy code (which may also help with partial rewrite later if it's judged to be necessary): golden master tests. Google it.


Here is something you might like:

https://github.com/NARKOZ/hacker-scripts

;) thank me later


I highly recommend the book "Working with Legacy Code". It gives you a step-by-step guide for how to work with legacy code without breaking things.


Making small changes incrementally instead of talking about what could be is a much more practical way of leading them towards sanity. Very common situation


Start with writing lots and lots of integration tests based on the actual end-user functionality. Skip unit tests and focus on testing the black box.


> I've already made some comments about rewriting it and the response was basically "ok".

Response from whom? The previous developer?


Alright I'll bite, do you work for Zenefits?


That thing you call “mess” is in fact bug fixes, late to arrive business logic, and tribal knowledge.


When time estimates become deadlines and the politically correct "Didn't we agree that it will be done by day X". Also, needs to be built by Wednesday and we will have the specs for you on Friday.


Refactoring and rearchitecting is a skill that is very valuable to develop and quite transferable.


i'd say, introduce automation testing to help to ensure that quality is maintained and improved. Then try and do what you can for a few months. If it feels like a headache everyday after 4 months look for a neww job


10k loc is small enough to refactor by yourself in your not so busy work hours


Here's my favorite talk on how to test, pin, and fix broken, weird, and ugly legacy code.

It's called "Therapeutic Refactoring" by Katrina Owen:

https://www.youtube.com/watch?v=J4dlF0kcThQ


Been there, done that. Here's the TLDR approach:

Perform small, incremental refactoring.

Spend 2/3rds of your time adding / updating tests.

(If you can,) involve the original author in code reviews.

Figure out what the original author did right. If the project appears to work, something was done right. Make sure to maintain the right design patterns, libraries, frameworks, testing tools, ect, as long as you can.

Honestly, refactoring is my favorite part of programming. Taking something that "almost" works and turning it into something that's industrial strength and maintainable is what gives me the most professional joy.

(Edit) The last time I was in this situation, the product had very visible problems. I had to put my foot down and declare that they had to be fixed, because no one would use our product if they weren't fixed. If you can point to a problem, like hard scalability limits that management agrees will become an obstacle to company growth, that's your way to get buy-in for refactoring.


My five cents: maybe try and work on setting up testing for the project?


If you can't change your organization, change your organization ;)


Do not attempt a rewrite.


I'm curious what language is used in the project ?


You sound like a novice programmer.

If it works, don't fix it.


This code is not written by me syndrome ?


Are there tons of global variables?


I recently rewrote a flashing tool that was used for programming engineering samples (which there is an extremely limited supply of) and production equipment.

Literally everything (and I mean everything) in the old implementation was a global variable that would just be recycled throughout the runtime of the program. Excluding loop counters, you could count the number of localized variables on two hands. The call-and-response serial communication implementation was a pseudo-state machine scattered across a shitload of global variables, and it ran entirely in the UI thread. When variables didn't have useless names, they would often have the same name as another variable, just with the capitalization changed.

Software development is hell.


At my work it's a few hundred globals, often times used in a few hundred different places scattered over, again, hundreds of files.

To make things more fun there rarely is a function with less than a 1000 LOC, written in a Winforms Button_Click handler. Less than 10 functions I've seen take parameters and return a value, almost all rely on globals being set and set some themselves in return.

After the manager and main author of the software asked me to build a new UI and leave the underlying code untouched I decided to leave the company and work on someone elses messy code


How about returning a reference to a static variable that changes when the function is called with different parameters. [C++]

I can't imagine why he thought that was a good idea.


Write the tests you need first.


I've struggled with this a lot. If you want to talk over the phone or via discord I would be more than willing to be a sounding board. :)

> Recently I joined a relatively small company (50-100 employees) as a medium level software developer. For a while I was very excited about this opportunity. 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.

Yep

> 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).

CACE principal at work.

> I started "repairing" the project but there're no tests to check whether my adjustments are correct.



Start writing tests, start motivating other contributors to write tests. Focus on the code that matters.

> I'm so frustrated and depressed by it. The job basically turned into something I hate - I'm just rewriting the code!

You're going to need to work through this and either change your mindset about this or start looking for a new job. I challenge you to do the former and not the latter. This is how 90% of the projects you're going to jump into are going to be. The real test will be whether or not you can thread the needle and make valuable improvements to the code while also delivering business needs.

You can't let this job get you down. I know... it's your vocation, but most of the time people treat their job like a sweet paycheck. Instead you're going to need to treat this as a challenge if you want to get through it. You've identified the problems, now you can either continue to be negative and wallow (bitch) or, attempt to offer solutions to fix them.

The challenge is not so much technical as it is cultural. You'll need to be able to communicate the value of the changes that you want to make. You'll need to change the mindset of the stakeholders of the people on the project to let them know why integration tests are not unit tests. Why, separation of concerns is important and how you plan on doing that. Otherwise, you won't be able to create change and this is the cultural challenge.

The bad news is that this isn't necessarily the exact job you signed up for as an engineer. The good news is that you'll be able to build new valuable skills that are often highly sought after. If you feel like you're a "better engineer" than the current engineer that built it, then you need to help elevate that engineer and the other engineers around them. Otherwise if you become depressive or negative on the job, you're going to have a major negative impact on the rest of the team.

> 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.

Welcome to business life. This is how it's going to be a lot. I'm sorry.

> 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.

No, don't go in and blame the person who wrote it. You always need to "consider the context". It's easy to point at someone's code and call it shit, or to say that a project needs to be rewritten. It's harder to reverse engineer the "why" of how it got to where it is. If you can identify the "why" then you'll have more insight as to how you may be able to fix the problem or address the aspects that may have caused the problem.

> I've already made some comments about rewriting it and the response was basically "ok".

"ok" or in other words, "how are you going to deliver business value for our team". Show them data, show them why having easy to maintain code delivers business value, then people's ears will prick up.

It's your job to manage this problem. If you have the necessary insights, it's on you to fix that. Otherwise move on and don't let it kill you. I know, my job is very important to me as well, you can't make yourself miserable over something like this. It will be far too prevalent in this field.


Fix it


> I started "repairing" the project but there're no tests to check whether my adjustments are correct.

First you need to discover all associated business requirements. You need to create all the missing tests, but you cannot do that without knowledge of the business requirements. Get these requirements and get them in writing. Find them. If you cannot do this stop. Stop now.

> 

I'm so frustrated and depressed by it.

This is ultimately the difference between a newb, a mid, a senior, and a super duper ninjitsu rockstar.

A newb will just accept bad code and work on top of it with their own flavor of bad code. A newb doesn't know any better and everything is frustrating all the time even if the code were awesome.

A mid has just enough experience to be dangerous. Sometimes a mid suffers from Dunning-Kruger Effect in that their personal perception of the existing code isn't how they would do it and their way is soooooo superior. The mid level developers who don't suck prioritize tests and security above everything else. They know they aren't fabulous architects, but they also have some idea of where code applies known bad practices and known best practices and can work from there. A good mid isn't going to rewrite the code base, because the risk is too high. Instead they will make the most minor of best practice corrections confined to the areas they have to touch anyways.

A good senior understands concepts like automation, graceful degradation, progressive enhancement, scope, recursion, and so forth. They know how to make the code work for them. It isn't about writing a few lines of code to solve a single business requirement. It is about writing a few extra lines of code to allow the current business requirement to scale to the next thing so less work must be performed in the future and less regression occurs when the business direction shifts.

The super awesome developer takes automation to a whole new level. Documentation is automated and therefore always up to date and accurate. Debugging tools are already embedded as part of the validation process so that when validation fails you spend less time on research or resolution. Dependencies are few and well managed, because nothing pisses people off more than corruption from outside the code you own. In the end top developers value the time of other developers more than anything else and will write their code accordingly. It isn't so much about the readability of the code as it is speed of maintenance cycles moving forward.

If your job is actually important to you then you will find an acceptable level of risk that you are willing to own and have the time to practice against. Getting this right shows that you care to do the right thing without making an ass of yourself at the company's expense.

https://en.wikipedia.org/wiki/Dunning%E2%80%93Kruger_effect


Hi! I have almost two decades experience in software development, most of which I spent fixing projects like the one you mentioned.

TLDR: this is hard

The biggest mistake people make (yeah, I made it too) is to quickly loose credibility with stakeholders by not providing visible value or by increasing failure rate.

Before you even start the project, you need to communicate with stakeholders and discuss what are their problems and how you are going to address them.

You also need to get very familiar with the codebase. You need to understand various patterns that are hurting it, what is their effect (are they just cosmetic problem, are they hurting reliability? maintainability?). You then need to create a step by step plan that will address those problems starting with removing the problems that are causing biggest issues, are prerequisites to resolving other issues and are least risky to deal with.

Remember, on any project like that you are spending your trust you have with the stakeholders. The more trust you have and the less expenditure rate the longer you will be able to work with.

Try to figure out how you can bring some real visible improvements at the very start of the project to build your credibility and then a string of even small successes along the line so that stakeholders are kept in positive mood.

Plan hard to avoid failures as the mood may change quickly to "why are we even doing this" when stakeholders are in hot water with their clients.

Choose KPIs and measure them throughout the project to give visible feed on your improvements to your managers.

On the technical side, it will be critical to have good development process. Automate delivery mercilessly early on -- this is key to keep failure rate in check and allow you to work at required velocity but more importantly, it is a visible improvement that you can show to your customer to build your credit of trust.

Touch the code as little as possible. Only try to fix the big problems that you have identified. If you start messing around too much you may introduce failure when fixing stuff that wasn't worth the risk and you also risk alienating other people that are working with the code base. Remember, you are making everything more difficult for them already, don't make it more difficult than necessary.

My most recent project was a mess of hundreds of components, 400k of extremely low quality code. Instead of jumping right away into fixing the code I have spent time talking to stakeholders and to what their problems were (not what I was thinking). I then quickly addressed their problems. The existing team wasn't prioritizing it highly because they didn't even know it was important... I then observed that with the existing development process I will never be able to deliver this many changes with acceptable failure rate, so I started by working on automating build, deployment and testing. Early on I identified that the high complexity of the project meant people were having hard time doing anything reliably. So I quickly prioritized reducing complexity over fixing bad architectural patterns. This helped my peers working on the code and further helped me fuel positive image of the change.


I've been there. Many times. To a point where I started introducing myself to new team members as "The Code Janitor".

> I started "repairing" the project

Don't. Unless your immediate supervisor requests you to do the repairs. These type of efforts largely go un-noticed or un-rewarded, and only cause you stress because you care a lot about the work and the codebase. Sometimes, you even get punished for trying to do the repairs.

> 

I'm so frustrated and depressed by it

I know it's hard, but try not to take it to heart. There's a life outside of work, and don't neglect that at the cost of your work ethics. Have to leave you with this quote from https://workplace.stackexchange.com/a/83319/12527

> There is an incredibly rich world out there outside of work, and my sense is that all too many people get caught up in what happens within the office walls, at the expense of simply being, living in, discovering and enjoying this world. There have been some interesting survey studies of people on their death beds, and what they wish they had done more of when they were younger and healthier. Hardly anyone says "I wish I had spent more time in the office." I keep this in mind every day on my way to work, and it helps me keep my internal compass focused on the things that really matter, so that I don't regret not dedicating enough of myself to non-work-related pursuits years from now, when it may be too late.

RELATED: Try to achieve the 'Zen-like' state that the dude in office space gets into, when he get hynotized and does't snap out of it. https://www.youtube.com/watch?v=Dp7EUUVdrt0


Should I just go and basically say that this person did a bad job?

No, because it’s not true. This person was operating under constraints you don’t understand with incomplete and constantly changing requirements. You would not have done better in that situation.

Other professionals do not stab each other in the back either. You will never hear doctors or lawyers do it. If you have a beef with this person keep it behind closed doors.


One thing I haven't seen mentioned is drawing a graph of dependencies using Graphviz. It's immensely useful for my current task - removal of two obsolete, fundamental variables from a convoluted permission system spanning a very big project. As a bonus, managers love it because it offers visual feedback.

Normally, unit tests would be the solution, but my ticket spans more than 10 modules (files), some of them suffering from the God Class syndrome. It's infeasible for 1 person with only moderate experience in unit tests to write enough tests before he's fired for not showing results. But it made me realize how important having unit tests in place is, and I made a resolution to write them for my future tickets spanning a single module.

Graphviz uses a super simple DOT notation that is perhaps best explained through showing the gallery. If you click on a graph image, it takes you directly to the source. https://www.graphviz.org/gallery/

With a big enough graph, GUI tools like Dia or Astah.net just don't cut it anymore. Graphviz lets you focus on the only thing you care about - content, and the visualization algorithm takes care of positioning. Because the format is a text file, it's a very good fit for git, you can attach a modified graph to each git commit.

Graphviz is also a great fit for vim. For example, to color all edges leading to particular node, you can type:

:%s/-> ParticularNode/-> ParticularNode [color=red]/gc

Or you can manipulate lists of nodes or edges with VISUAL LINE select mode and type :sort. Lots of possibilities. It's easy to extend a graph copying and pasting modules (subgraphs with cluster_ prefix ) or anything else, and it supports attribute inheritance so you can cut down on redundant lines. Just declare a node style, for example:

node [label="update"] update0; update1; update2; update3; update4; node [label="create"] create0; create1; create2; create3; node [label="\N"] // this resets node labels to default behavior

This way you can have the cookie and eat it. All methods of endpoints are uniformly named, but it's not ambiguous which one is which.

And the kicker - graphviz was initially developed by AT&T, so you have a BIG NAME to back up your tool choice.


Iterative refinement. Start with the most problematic parts of the code and work from there because you don't want to have to redo code because it didn't mesh with the higher priority fix. Unit Test extensively!


you can't just say someone did a bad job as management is probably also to blame. you have to be more tactful about it.


you can't just say someone did a bad job as management is probably also to blame. you have to be more tactful about it




Consider applying for YC's Spring batch! Applications are open till Feb 11.

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: