Ask HN: I just inherited 700K+ lines of bad PHP. Advice?
- getting a handle on the code base - communicating 'progress' to the client - not losing the will to live
The software is based on vtiger, an open-source CRM that has a (deserved) reputation of being incredibly badly written, that has since been badly hacked apart by several different companies with wildly differing ideas. My client currently have 150+ installs and 150+ angry clients.
Words fail me trying to describe the state of the software.
- no niceties such as MVC, ORMs, a DBAL, or a modular design - all DB queries are inline SQL, with tens of inner joins on most queries - dizzying call stack, yet reams of copy+paste code
The best part: the code will often query the DB and execute PHP code contained in the response, or load and run arbitrary files and modules as dictated by parsing particular DB fields. The one page I have studied in detail generates 105 DB queries in the simple case.
The DB itself is even worse. There are over 600 tables, as well as views, custom functions, cascades and (but of course) triggers. There is no consistent naming schema, very few explicit foreign key references (despite being heavily, heavily entwined) and I have already discovered several tables that don’t have primary keys, but are referenced by exact string matches on things like date stamps.
I wont mention the table-based HTML, javascript, lack of version control etc.
I’m not sure if its even possible to give relevant advice (besides perhaps ‘run screaming’), but if anyone here has come through a similar situation and has any advice to share, I would be deeply grateful.
Help me HN - you're my only hope. (PS. 2K char limit sux)
77 comments
[ 3.7 ms ] story [ 223 ms ] threadI think you should start figuring out where it forked from the original code base & generate a diff from that. Then throw a bunch of tools, from Cacti to Xdebug/APC's control panel, to get a good handling of the current benchmarks. Get the code into a SCM (probably git), then start tackling current bugs & feature-requests - BEFORE you start tearing down to the baseboards.
After a month or two of that, you'll see the first pain points. Going after the pain points one at a time, rather than all at once, will keep your client happy and you sane.
I would look at migrating the clients to another product that works, or even a less forked version of vtiger.
Software Engineering is serious business, there's bugs, new features, maintenance, testing, etc. They failed to manage their code. You need to be realistic that with a team of 2-5 people it could take years to fix.
It might be best to put it out of its misery if they can't hold off their clients demands and buy you the time needed to rebuild it.
Unless you're can't-pay-your-bills broke and there are no other jobs in your area, the correct response is "I'm sorry. This can't be fixed. It needs to be scrapped completely and replaced with something maintainable. If that's not an option, I'm going to have to resign."
Even if you manage to fix it, it'll still be a pile of crap, and you'll never get the credit you deserve.
On the other hand, if you don't manage to fix it you're going to get blamed.
Zero credit on the upside, major blame on the downside. There's nothing for you in this but pain, my friend.
I've turned around a few failing projects and rarely is the problem technical, most of the issues start with poor management.
You need to sit down with them and explain that just isn't possible, then explain clearly what is possible, and what the options are (painful rewrite in parallel to maintaining the old software, or painful and slow refactoring), and just how much it's going to cost them. They aren't going to like either option but it's better for them to understand upfront exactly how much of a problem they have, and that the problem was not necessarily with the previous contracting companies, but with their codebase.
I'd suggest that correcting the DB is one of the last things you can do, especially if queries are scattered throughout the code instead of in functions. You could attempt to abstract it with an internal API and as you update the codebase replace with calls to the API. Once fully abstracted, you can then focus on getting the DB corrected and only need to modify the new API functionality.
As to the code itself, sit down and map out all the verbs and nouns in the system. If you have a Contact noun, what is the definition of that role and what verbs can be applied to it or what verbs could it do. This gives you a good map for creating functions that can then be used to replace existing inline stuff.
Triage the worst bugs or performance bottlenecks and see if they are particular to a noun and/or verb, which should give you an obvious starting place to begin refactoring. For emergency hotfixes and such, feel free to just tweak the existing crap code but otherwise try and work on your functional units to get ahead of the game.
And always remember, pimpin' ain't easy. ;)
As you're doing so, move all those queries into one big fat DB class, and when you spot groups of related queries, split them out into their own classes.
The next priority should be to get rid of the PHP from the DB - if need be create another huge class with a zillion if-else statements.
You need to modify the code to simplify it. You don't need to improve the design, you just need to dumb it down until you can understand where all the parts are. Stabilise, simplify, then refactor.
I would also recommend Working Effectively with Legacy Code By Robert C. Martin.
Good luck!
This will allow you to be more aggressive when replacing crap code with new functionality.
Given this huge mound of code, this process will be quite a drag, but it will pay off in spades in the long run. Trust us.
I would talk to the client about focusing your effort on helping them transition--a small piece at time--to a sane architecture. If they aren't open to that, they aren't your client. I've been a business leader in a position where we had to make really tough and painful decisions about coding projects gone awry. I don't envy their position, but continuing forward with this monster does not seem to be in the long-term interests of the company.
If it's maintenance/bug fix, I'd suggest starting by writing tests and fixing things on a day to day basis.
So, for all small tasks that you'll have to do with the code base, just analyse the safest way to tweak it. Most often than not, you'll see that it's just changing a couple lines. If you need to add new features, just code them correctly in another part of the program.
And before you know, you'll understand the code-base. But, tests are really the most important thing here. Don't try to refactor if you can't make sure you're not breaking everything.
Personally if I were in your position I would explain to the client that I'm happy to temporarily fix some bugs but long term the system needs to be rewritten. 700k lines of code is a lot, but the way you've described it I get the feeling most of that code is needless. Depending on what the system actually does you could conceivably rebuild in a few months.
I've come to believe that the skill of not rewriting from scratch but forcing yourself to slowly refactor (as per Martin Fowler's definition) existing systems into a proper state is one of the most important skills you can develop.
That way, once you've refactored most of the system (which includes adding tests for all the important functionality) you can indeed confidently rewrite everything. If you do it any sooner than that though, you're in for a world of pain.
The moment you start touching the code, you are going to start being blamed for the nightmare preceding you. It could even affect your career if future employment researches where you worked previously and gets told you made the mess in the first place.
If the former, has the incredible scale and scope of this been properly identified, addressed, and acknowledged? Are you guaranteed anything near the resources to (try to) accomplish this (including your own time, without traipsing far into overtime)? Is the current state documented sufficiently to obviate any and all future attempts to blame you?
If the latter (more likely, I suspect), well... I guess the simplest question is, do you have an agenda and an exit strategy that leaves your career intact? (And your health...)
Maybe, given the particulars, this is a real opportunity for you. But that's not spelled out at all, nor obviously implied, in your post. And given that this situation was allowed to develop to this extent in the first place, and that you have angry clients to deal with, right off the bat, it doesn't sound promising.
Do you like playing the role of unacknowledged hero who falls on his sword and is cursed by his clueless fellows, while some other protagonist goes on to get the girl?
There is a lot of downside, here. What's the upside? Do the organization's goals and commitments match your personal ones?
One of the most important things is just to manage expectations - it sounds like you've got a huge task ahead of you and people will underestimate how long it will take you to fix stuff.
It might also pay to just focus on getting the software into a maintainable but ugly state.
2. Make sure there is some workable strategy for deploying and testing the code.
3. Ask somebody to provide you with a list of the changes, or else try to create some kind of diff against the original version of the code. If you can see crazy stuff here then find out who did it...
4. Ask somebody what the biggest bugs are? Which things are causing clients the most problems?
5. Try to establish which convention is 'winning' in the codebase. But you might want to create a more sensible convention which will allow unit testing (start this immediately!)
6. At this point, ask if you can hire people to work on this with you as it's a big problem, and you need to free yourself up for the rewrite.
7. If that isn't possible then leave. You have done enough to make your CV better and a company which passes you something like this does not care about your career.
1. Identify a small and easily separable piece of code (what you woud call a component in a normal system.)
2. Write tests covering every (important?) edge-case of the piece of code you want to rewrite.
3. Mercilessly refactor until it's nice and squeeky clean.
4. Lather, rinse and repeat.
And of course, make sure your client acknowledges that it's a giant clusterf... and is on board with you pulling the system out of the stone age.
Also, if you want to make life a bit more interesting for yourself, get the PHP code's AST and programmatically rewrite existing code to shared conventions for kicks.
"Refactoring" is not the tool for the job, although it's a nice sidearm.
What OP needs is the big gun, Feathers's "working effectively with legacy code": http://www.amazon.com/Working-Effectively-Legacy-Michael-Fea...
As the title hints, it was written specifically and expressly for the "I just got a huge amount of complete shit of a codebase shoved unto me, how do I survive". Just check the TOC of part 2 (the meat of the book): http://my.safaribooksonline.com/book/software-engineering-an...
> And of course, make sure your client acknowledges that it's a giant clusterf...
That's hugely important. No promises of delivery, and that the client understands it's not a cakewalk.
2. Set up a development environment and deploy the code there. Get it working. With code that large (and with the added wrinkle of executing code out of the database) changing things is going to be a nightmare of unintended consequences. Getting a testable environment up will let you find those things and help you understand what it does.
3. Get it in version control. This should be number 1: Before you make changes get a baseline of where it was.
4. Find a bug that exemplifies the nastiness of the whole situation and make a fuss. Let everybody know why this bug is so bad and what caused it. This will give your employer a concrete example to look at when you say "this code is shite". Harp on this bug.
5. Fix that one bug. Roll it out. Be a hero.
At this point you'll have a good base line, some credibility, and the organization will understand what a mess they've got. Now you'll have to figure out what you want to accomplish: keep it limping along? Improve it? Rewrite? The above steps will get your feet under you.
Second, I suggest applying as many tools as you can. A modern version control system, of course, and keeping any version control history that you inherited (although it sounds unlikely).
A powerful IDE might also let you start cutting out crap immediately, so try PHPStorm or Eclipse+PHP (or both!) and see what they can tell you.
And start writing tests as you start making changes, because you'll likely break something seemingly unrelated when you start changing things.
* NEVER modify existing one. Once you change one line of comment, you own all the code and problem from that point.
* If rewrite is not allowed, then ask huge pay raise for this work. Basically it is not about money, it is about bring everyone on the same page on the status of he existing solution.
* If the above does not work out, prepare to switch to another project, or quit the job totally.
But that situation is different from the one we discuss here.
I do not know more information about ohmygord's project, but I basically want to point out to consider non-technical side of it. For example, people in the same team may not technical, and/or think maintaining existing solution is simple. I was in similar situation before, I was lucky to happen to select right strategy to deal with the situation.
Run away from this. Trying work with this code would make you stressed and frustrated, which will have a significant negative impact on your productivity.
If the company plans to add features to this software, they should hire more than one developers and perhaps rewrite it from scratch.
Edit: Also send your boss link to this discussion :)
* This is not best situation to be in, but if you learn to deal with this and emerge from it. This experience will make you so much stronger. So be ready to quit, but do not quit too early.
Good luck!
(I replied earlier, but the above two points are so important that they worth a different post.)
1) Study how the software is actually used and design the "ideal architecture" (this may be a moving target).
2) Get the software into a version control system.
3) When a section of the code needs work, first write tests that pass for the current functionality of the module but fail for the behavior you're trying to fix.
4) As you repair code in step 3, also migrate the code "towards" your preferred architecture ... this is going to be a very gradual process so don't try to complete it in one step and use your tests to verify you haven't broken the system. This is also a good time to start inserting patterns like MVC/MVP as it will help. - http://c2.com/cgi/wiki?TestEveryRefactoring
5) When you've found "reams of copy+paste code", refactor that code into utility classes (files, whatever). - http://martinfowler.com/refactoring/
6) Establish processes for migrating the database both forwards and backwards between versions (you'll need a rollback someday).
7) Treat the database schema as source code and refactor it as you work. It sounds like you're a long way from being able to use an ORM, but have a plan for migrating the database towards the day you can. - http://martinfowler.com/articles/evodb.html
8) Get the PHP code out of the database ... that's going to be painful but worthwhile.
9) Get some help! I've used the Sonar source code quality analysis tools on Java projects for years. There's a PHP plugin for it here (http://docs.codehaus.org/display/SONAR/PHP+Plugin) and it will help you determine what areas might be worth targeting. It also helps by establishing style and practice rules that will help get a team coordinated.
One of the hallmarks of a project like this is that coding styles changed dramatically during the project's existence - Establishing a style guide (including patterns and forbidding anti-patterns) can be very helpful.
So in short ... don't "run screaming" but rather sit and think when you feel overwhelmed. If you can solve a complex problem when writing source code, you can also solve systemic problems.
Good luck!