Ask HN: Started a new job and their existing code sucks. What to do?
I started this new job and the current code is so spaghetti that it's almost an Italian restaurant. No good practices are being followed, there are no tests, huge components that should be modularized, no organized architecture at all.
The current developers working on that code aren't very experienced and, even though they acknowledge that some refactoring is necessary, they don't think that's a priority (and that we should focus on bug fixing). However, the thing is so messy that they've been spending the last few months just bug fixing - while refactoring the code would have solved most of their problems.
What would you do? Just don't care and keep doing what you're told to do (bug fixing/closing Github issues) or actually starting refactoring (or rewriting it - sometimes I feel an urge to rewrite the whole thing from scratch) on your own?
This is my first time starting a job with such a bad existing code, so I have no clue about how to proceed. I'm just feeling very depressed having to work on that everyday. :/
65 comments
[ 3.1 ms ] story [ 143 ms ] threadQ: How do you eat an elephant?
A: One bite at a time.
Talk to your boss. If you feel strongly it needs to be done, explain why, and what the benefits are. You need to be able to quantify and qualify this so do your homework and sell it.
Contrived example: Hey Manager, we've broken this experience 12 times in the last year, so I'm writing unit tests for it, so we can't do it again without knowing. It will take me an extra few hours on this issue but the payoff should be worth it. I think we should do this for everything.
In the meantime, leave everything you touch better than when you opened it. Start teaching the practices you want to see.
This is a good book on the topic refactoring a large code base with no tests.
https://www.amazon.co.uk/Working-Effectively-Legacy-Michael-...
This way you will have some better assurances when you change things.
There is a great book on Legacy Code by Michael Feathers that I would also recommend.
And as far as refactoring goes, take things 1 part at a time. Don't try to refactor everything all at once.
- just make the function / class you're working on nicer, preferably just a small function. Apply some common quality standards to it, clean code, nice docs. - repeat this for a while, and you will have a nice set of examples and guidance for the other team members by setting an example.
Personally I am a huge fan of making that one place where you found the bug nice, adding a simple test case and moving on to the next thing. It adds up over time and does not hurt in terms of time involvement/budget since you already had to invest a larger amount fo time to find the place bugging out.
Considering that you will face this probably with your next employer again, deep breath and just enjoy the actual engineering work!
Personally I think it’s irresponsible to hire new employees before having a maintainable project. It’s much easier to onboard employees and delegate to them when they have a safe dev environment and testable projects to break while they learn.
Gradually that test suite will accumulate enough stuff that all OP has to do is walk into work, pull the latest, and make a few easy fixes before they finish coffee. They will look like either a hero or a monster, depending on how the politics work out. It's risky, but either way something will have been stirred up.
Have you ever been in a situation where you think "Gee, this is a mess but we are swamped here! Let's hire more people to help fix this" and then the new hire(s) joins the team and proceeds to bad mouth everything instead of helping to make things better? I have and it wasn't lack of communication or openness.
It's an attitude I can't empathize with.
- hiring people for unmaintainable projects is literally the default for the majority of companies as everyone starts out small without directions and either fails or becomes something big. Legacy code is basically everywhere, clean code is the minority.
Most code is like this, it's semi-painful to work in. Once you have been working on it a while you'll be more used to it.
I would spin it more like this. Hey if we refactor/rewrite this we can improve it and knock out these bugs in the process.
It may be hard to convince the company to spend money on refactoring let alone writing tests. But you can always ask in a positive way to start on parts of it.
You get paid the same, be positive, fix bugs/issues, improve things where you can. Keep making positive suggestions.
As a developer you're almost always going to feel the urge to rewrite every app from scratch. I expect the code is making them money and they just want to keep things moving for now.
At some point there is usually a rewrite from scratch, maybe that can be your project.
Good luck with your new gig.
> The Boy Scouts have a rule: "Always leave the campground cleaner than you found it." If you find a mess on the ground, you clean it up regardless of who might have made the mess. You intentionally improve the environment for the next group of campers. Actually the original form of that rule, written by Robert Stephenson Smyth Baden-Powell, the father of scouting, was "Try and leave this world a little better than you found it."
> What if we followed a similar rule in our code: "Always check a module in cleaner than when you checked it out." No matter who the original author was, what if we always made some effort, no matter how small, to improve the module. What would be the result?
http://programmer.97things.oreilly.com/wiki/index.php/The_Bo...
Apply it as you go. Piece by piece. Perhaps encourage your fellow developers to adopt the practice.
People would stop being willing to review his PRs, because they'd always contain off-topic changes?
If the company is fairly new, then bad code is kind of expected. There is a rush to get a product out the door, so shortcuts are taken. Given time for things to settle v2 will be better.
If its an established company or the code is legacy you can fix little bits as you go along. If documentation is lacking, write some - maybe even set up a wiki and bug tracking system if need be. If comments are lacking add some.
If you can demonstrate the bad code is costing the company more in man hours (and therefore money), then maybe propose a a series of refactoring projects in order to help improve the code and increase efficiency. I worked at a company that would have refactoring parties once a month with pizza and chipper. On the last Friday afternoon of the month we would review 10 random code files changed in the last month and tidy messy code, add comments, docs, unit test etc.
Suggest the company adopt some coding standards if they already haven't. Ditto code reviews and unit testing.
Of course not all companies are receptive to such changes. Some take the view that if the product works, why wasted time on frivolous extra like docs and code reviews. Sometimes when the code has been primarily the work of one person they might not take kindly to criticism - even if valid. In those cases all you can do is just do is to write your own code as best you can and not worry about what goes on around you. Bad code is a lot more common than you expect and life is easier when you accept that.
Also consider your teammates built that code. How do you think they felt when you told them it needs a whole refactoring? Since you cannot refactor it in secret, you would need to get your manager’s approval to take on such a project. He is going to ask your teammates, who built the code and whom he’s known longer than you, for their opinions. So it’s important to have their support in refactoring.
It seems like a needless risk. You have a low chance of success (first, convincing your manager, and second, successfully refactoring the code), but a high chance of loss (alienating your teammates).
All of this argues in favor of having an overall architecture that can help a team member grasp these complexities. Maybe there is one that OP cannot see. However, edge cases also cry out for automated tests. Whether or not a codebase has automated tests is explicitly clear from the first day.
> So it's important to have their support in refactoring
This is absolutely true. A software team is made of people and trying to drive change without respecting people's need to be persuaded is both rude and doomed to failure. That persuasion is hard and I wish I had better advice for OP than to read books by the Harvard Negotiation Project.
> How do you think they felt when you told them it needs a whole refactoring?
Depends on the teammates. I've built a codebase that was a pile of spaghetti before and I felt terrible about it and left. So there may be people who agree with OP that the codebase needs significant improvement. There may be other people who, in the style of https://danluu.com/wat/, never imagined that things could be better.
But one thing that the current teammates have in common is that they all have not left. The people who care most about clean code are the people who self-selected out of the team already.
> It seems like a needless risk.
So is making any change to the codebase without tests.
Sounds like they're more interested in make-work and job security than creating a good product. You may wanna get out of there.
A lot of the advice I've seen here relies on the rest of the team wanting things to get better. That's not always the case. Many teams are very proud of their unmaintanable, untestable spaghetti. Some see it as a of badge of honour that nobody else can figure out something as basic as where model validation happens (in that specific case, they marshalled all their model validation into the controller parent class, and things got worse from there).
So the first thing you need to do is figure out how open they are to changing. If they're happy with what they've got, and they don't want to change how they write code, then there's not much you can do. Cleaning as you go is a great idea. But even if there's only one other person on the team and and they don't want to change anything, they'll be generating mess twice as fast as you can clean (making a mess is always faster than cleaning).
That's my advice. Try to probe if they're willing to admit they even have a problem. Start with floating the idea of writing tests, or implementing a style guide. If they don't even want to do that, there's no way you're gonna get anything bigger out of them. At that point, you gotta decide if this job is right for you.
Of course, the other side of the argument is that it's not unlikely the head of the company is the guy who created this problem, so maybe looking up one of the board of directors, or an investor, is a better idea.
Are you assuming that OP works in a small SV startup? Most jobs are in companies where that isn't plausible.
OP did not say a single thing about the size of the company, nor anything to suggest that they have been hired in a significant management role. It's entirely possible that they're going into a junior role in Amazon for all we know. Your replies, in contrast, don't even admit those extremely-common possibilities, and I find that omission baffling.
So, confused, I'm trying to think of explanations, such as that all your experience has been with smaller companies... or that were your born into upper-management Jeff Bezos is a golf acquaintance.
Yes, there are bosses who don't want awkward info. At NeXT, Jobs didn't want to know (in those days) he just wanted stuff done, so even though his N.A. managers pretty much knew the European manager was not telling the truth about his spectacular sales success they didn't make sure the information got to Jobs, since he didn't seem to want it. Worse, the European staff regulars who were asked to do the dirty work and did know there was a big problem (if only because they say huge stacks of computers piling up in warehouses collecting dust, say) didn't see fit to inform Jobs. The company failed because the European manager was committing massive fraud to fake sales. (Without that, it might have failed for other reasons but that's why it failed.)
But Jobs was unusual in more than one way (and did learn that lesson.) Why on God's earth assume that upper management never wants bad news? The truth is that upper management is often very well-aware that frequently middle managers, and low-level managers tend to serve themselves even to the point of putting the whole company at risk and see it as a big part of their job to prevent that; but they don't know how and where until they're told. The fault with upper management is more usually that they assume that some floor employee would have knocked on their door if anything odd was going on; but as the discussion here shows so vividly, that's by no means always true.
Yup, again, there are some bad bosses who are going to drive their companies into the ground if they can, but it's a dereliction of duty to assume that. If you're taking money from a corporation, twiddling your thumbs while it starts to goes into a steep dive, or continues one, isn't ethical.
Of course, if you go over a boss's head, you absolutely have to have done your homework, and to be right. It's not something to be done impulsively, but it's a duty.
Books such as "The Hard Thing About Hard Things: Building a Business When There Are No Easy Answers" by Ben Horowitz take great pains to convince those heading or managing businesses to make sure every employee takes responsibility for what happens to the company as a whole (from the employee's action and inaction) if that boss wants their company to survive. I could name many others.
Second, this might be a first for you, but a new person coming in and deciding that the current code base is unsalvageable and needs to be completely rewritten is pretty much a cliché in our industry.
So what would I do? Like cheshireoctopus said, you can leave each bit of code you touch a bit cleaner than you encountered it. You can also try to get approval to start writing tests — having a test suite in place will first help you understand the existing code through and through, and then provide a safety net once you get around to proceeding to the actual refactor.
Amen.
The problem isn't going to be the code - you can fix that. It's if management is actually ok with things changing. So often there's resistance to change for a variety of reasons, some good - some bad. Bad ones include "it's too hard" or "Well, this guy wrote it this way and we don't want to hurt his feelings".
At least they are into bug fixing. I've been at companies that were feature-focused 99% of the time, because the CEO and VP of Product had overpromised so much to clients and investors that we were always chasing some overly ambitious fairy tale. Quantity ruled over quality. When things broke or didn't scale, they acted like they had no idea why. It was always more more more. These situations are the ones that can't be fixed. Run as fast as you can.
But, if they are amenable to getting a plan in place and working on this stuff slowly, then it surely can be fixed. Bug fixing is important though, as I don't want to try to refactor broken code, only to break it more.
I had the same problem when I started working at a startup a few years back. The code base was a spaghetti factory, but that was the least of the problems. No one was prioritizing issues properly, management was aloof, the bugs being discussed in the product were huge oversights, deadlines were looming, but nothing about the trajectory was indicating that anything would be met. I left after a week, a year later, they went out of business.
So I guess the take away is, if it's just code, being new, you will always run into differences, poor code (maybe there were reasons for quickly scrambled code), oddities, but don't let that throw you off, maybe they need more people so they don't need to scramble so hard? Bringing it up, or butting heads will only keep you from progressing. If you want to stick it out, assimilate and just prove yourself through superior code/work ethic. But if the code is a symptom of a much bigger mess in the company, then maybe look for something else? It's tough, and we would really need to be there to understand the whole situation.
http://thecodelesscode.com/case/33
You might want to do some reading of books about how to promote institutional change from within. If you can read, and then get your manager to read, "Creating a Software Engineering Culture" by Karl Wiegers, that might help. Also, "Refactoring" by Martin Fowler is highly advised.
Regardless, maintaining good relationships with your coworkers, and specifically your manager is key. One key insight I have gathered over the years is that whenever possible, there are benefits to avoiding telling someone "this is a problem/bad/whatever and needs to be fixed" if it is instead possible to say "That's great, and it would be even better if..." The criticism in the first form invokes a immediate defensive reaction once someone hears "this is a problem" which may cause them to not hear whatever you say next, and can create a teacher vs. student dynamic (see Pavlov's work on negative conditioning) in which they may actively start to resist your suggestions, while the second form is more likely to cause them to feel good at having been praised and want to seek further approval (positive conditioning, which tends to be much more effective and produce more long-lasting behavior changes in most circumstances). Some research on self-serving bias suggests that people are much more likely to believe what you are telling them if you can phrase it in a way such that it sounds like praise.
Good luck!