A dilemma often faced by professional programmers is whether to work with what they have, or start fresh when inheriting others' code.
They say it's easier to write code than to read it (especially other people's code). Why is this?
- Unless the code is perfectly readable, it's challenging to put yourself in someone else's head.
- Every project has context and baggage that is often not documented.
- As a new developer you have the luxury to be a critic. The former developer had deadlines to meet, family crises etc. You will also, soon enough...
Because others' code is hard to read and understand, how do we objectively judge whether we should work with what we've been given, or rewrite it entirely? The question is important because we will soon own what we've inherited. You can only defend missed deadlines by blaming others for so long.
When reviewing a potential "fix it" project, ask yourself the following questions:
Are There Docs?
Developers don't usually like to document, plus out-of-date documentation can be worse than none at all. But the existence and quality of basic documentation (e.g. a README explaining how to install the app etc.) can tell a lot about the general code-quality of a project.
Is the Stack Appropriate?
Evaluate the tech stack against requirements and use-cases. Is the stack optimal for thousands of transactions per second and many users, but the app has ten active users and only a few transactions a day? Is your data's structure unpredictable or document-based, or is it stable and clearly relational? Does the database match your data?
Is the project a typical CRUD app, or is it something different? Was the stack chosen to solve a problem, or was it chosen because it's trendy?
Are Dependencies Up-to-date?
We're all human, and every project has out-dated dependencies. Our clients don't pay us to update their code. That said, is the primary framework one or three major versions behind? It's a judgment call, but certainly a very out-dated dependency tree is not only insecure, it makes the code harder to work with.
The fact that the former developers were careless about this may indicate they were careless in many other ways.
Is The Code Modular
Is code broken into reasonably sized functions? Is it clear what each function does based on its name? Are there God objects or functions? Are modules appropriately scoped/sized?
This alone can tell you a lot about the quality of the project.
Are Security Best Practices Followed?
A big part of security is ensuring dependencies are up-to-date. Various package managers may have facilities to audit your dependencies. These audits can help ensure there aren't serious security holes lurking in the dependencies. NPM has npm audit, for example.
Also check how sensitive credentials are stored. Are secret tokens or personal information hard-coded in the source code? Hard-coded, sensitive data can indicate ignorance or carelessness.
Test any places where there's user input, such as forms. Confirm data is being validated both client and server-side. Do API endpoints reject malformed data?
How Easily Reproducible is the Dev Environment?
How easy can you get up and running as a developer? Do you need magical incantations and undocumented packages installed on your host to get the app working in a development environment? If this is a major struggle, imagine what will happen when you really begin working on the code.
How Are Error/Edge Cases Handled?
Most developers (especially under deadline) focus on the happy path and figure they will come back to error cases when they can. This is the beginning of the end for a project, because under stress the developer never comes back to the code after the happy path is covered.
But error cases often make or break a project because as soon as real users interact with your app, they do things you hadn't planned for. That's just the way it is. When I code a feature from scratch I often write the tests for edge cases first. It helps me:
- think through everything that could go wrong (within reason)
- ensure I don't have to "come back" to error cases, because I already handled them!
Exception handling etc. is a topic unto itself, so I won't expound on when to handle exceptions and when to let the caller handle them, but I do strongly suggest you evaluate how the app handles exceptional cases. For example, are all HTTP calls assumed to succeed? If not, is there a broad and cohesive strategy for handling unsuccessful HTTP calls to external APIs?
Are There Tests?
Now look at automated tests. Are there any? If not, and the app is mature, run. Far away. Because not only will you have no idea what you're breaking as you work, it's a sign of poor craftsmanship.
If there are tests, execute them. Do they pass? Are they testing important things? Are they stable? Run them ten times, and you should get the same result each time. Evaluate the quality of tests and whether distinct units are being tested, how mocks (if any) are used etc. Most importantly, try to write a test for an existing feature. Is it clear how to do so from the existing examples? Is it easy to do?
Extra points if the project has continuous integration.
Does the App Work?
This may seem obvious, but run through various user flows and see if you can break the app. Is the app reliable?
Conclusion
It's often not clear-cut when inheriting a project whether you should work with what you have or start fresh. Sometimes only working with code for many weeks (or more) will tell you for sure, and by then it may be too late.
That said, spending a day carefully evaluating the state of a project before taking it on is effort well spent. If you're handed an app that simply doesn't work well AND it's hard to write a few meaningful tests, then you could be in for a world of pain. Estimate the time it would take to rewrite the app from scratch and be prepared to discuss with the client the current state of the project and the options if the project is in bad condition.
When you start a new project, consider that someone may be inheriting your project someday—maybe sooner than you think. It's easier to start a project right from the beginning than to rein in a wild one.