Is this code bad enough to warrant a rewrite?

Question

This is a web application for payment processing. I've been here maintaining it for nearly two years now, so I have a good idea of the problems with the system. About a half a year after starting here I started a rewrite in php/mysql that ended up not getting used (not because of good reasons like "it hasn't been tested" or "how do we know if it will be stable?" but "php/mysql are not microsoft and are therefore bad" and "we have to buy a license for mysql / we'll have to share our source code if we use it / no shops use anything but microsoft products" - those were thoroughly entertaining conversations. This ended up being a fully functional throwaway prototype. I wasn't happy with the end result (mostly with how the HTML was coupled with code, it would have been hell for a web designer to work with), but it worked and at least I gained complete insight on the input/processing/output of the entire system.

The current system we are using works, but is fragile and difficult to add features to without breaking something else or requiring lots of manual fixes down the line. But is it bad enough to need a rewrite in order to move forward and meet the requirements and features new potential clients? I think so.

Here's a list of code smell:

• Originally designed in Visual Basic in 2003 or earlier, no major design changes since
• No classes or any kind of object-oriented design
• 3700 line Module1.vb file
• Manual deployment of executables and aspx files
• Complete mess of HTML designed in 2003, under-utilized stylesheets
• New clients were created by copying and renaming pages and functions
• Copied function names, e.g. function1, function2, function3
• Highly coupled with SQL Server
• Redundant database connection->command->reader blocks
• Highly coupled with our current payment processor
• Lots of functions with 6 parameters (many byref because there are no classes)
• No unit testing, have to debug from the start of the application to the finish every time
• No consideration for security
• Coding style is completely inconsistant, save for what VB does
• Settings (including passwords) are hardcoded in compiled files (besides those I've moved to web.config)
• Comments are either dead code (found everywhere, sometimes up to 50 lines), stating the obvious, or non-existant
• Extremely fragile - during revisions, changing code in one place affects stability elsewhere.
• I fixed a byref parameter of one function to not always be empty (so it would actually put a value into the variable it was supposed to be getting, but who could tell if it was used or not?) only to have a block of code with an obvious flaw (that had been there for over 5 years) suddenly start working.
• No test objects - all test data is retrieved from live databases or copies of live databases
• No thought to database table design - tables contain useless and redundant data.
• Code highly coupled with database tables/field names/types/procedures
• Warning 279 Maximum number of warnings has been exceeded.
• No separation of debug code and production code besides if statements (had a huge error with this at the end of 2009)
• Many, many redudant SQL queries to update rows with session data
• Javascript file is completely undocumented and has functions and have useless and redundant names (e.g. testFun, validate1, 1a, 2, 2b, 2c, and so on)
• External documentation is just an overview. I've written decent documentation since then.

Answer 1

Don't rewrite. Refactor it bit by bit. It 'works', don't throw away that investment.

• Start by finding hard coded values and moving them into config files.
• Then, start moving all data access into classes, and stored procs, etc.
• Then, implement style sheets.
• Then, find whatever dead code you can, and rip it out. (those are fun days!).
• Then, find whatever duplication you can and start abstracting into classes, etc.
• Then, look at the beauty that is left.

The steps don't need to be in this order, obviously, but after each step, test, release, refine, refactor some more. Create tests as you go to ensure you don't re-break things! It's a long road. And in the end, none of the original code will remain. But, you get the benefit of being able to test incremental changes/fixes in a live environment with live testing. Rather than trying to create a huge rewrite, and dreading the "go-live" day because you don't know what will be broken.

As you are fixing bugs, and maintaining existing functionality, leave each page, function, etc a little better than you found it.

Rewrites NEVER go smoothly. You're trying to maintain v1, will building v2... and you end up in a race you can't win. You end up missing a critical feature. You end up with code that has a new set of unknown bugs because it doesn't have years of use and real live testing.

Answer 2

Are you asking the right question to the right people? It sounds to me like you already know what the answer is (or at least what you want it to be).

You have identified numerous (technical) problems with the existing source code, have even done a rewrite (in your own time? or did the company provide time?), but still management apparently doesn't listen. To summarize your argument:

• It's difficult to know where to make a change
• It's difficult to know how to make a change
• It's difficult to know what other functionality is affected (broken) by a change
• It's difficult to test changes
• An unnecessary amount of attention is required to keep the system running

What arguments has management given? Are many changes required? How much do they think a rewrite costs? How much risk do they think is associated with it? Can you convince them otherwise? How much time will they save in maintenance when the rewrite is done? How will the customer be happier after a rewrite (less bugs, better performance, ...)? What alternatives to a rewrite exist (keeping the old stuff, refactoring bit by bit, ...)?

And don't forget: How will your work satisfaction change? How strongly do you feel about this? If you are willing to change jobs to avoid working with the existing system, I think you should let them know. Of course you should be careful how you say this, you don't want it to become a pissing contest which ends up getting you fired, but it helps to let them know you feel strongly about this. Maybe something like 'maintaining a system like this is not what I see myself doing down the line'.

Of course, it is possible that you will just have to agree to disagree. But if their arguments are just wrong and nonsensical (nobody uses PHP? only MS is cool?), maybe it is time to move on.

[update] Just a warning to add: a rewrite won't keep you from having to support crappy code, it will just change old crappy code to new crappy code (hopefully less crappy, but you can't be sure yet). Looking back, and having learned in the process, your new code will look crappy too. You will only know how to do the rewrite correctly when you're finished with it. So proceed with caution.

Answer 3

I think the problem is not that the code is bad enough to be rebuild but the willingness of the company to pay more (on the longterm) by expanding that mess. You have to take more time for new features and maintenance and so they have to pay for your extra-time.

With a simple business-case you should be able to convince them to recode that application. Try to tell them that their code is outdated and will block profitable growth.

It really sounds like it should be rewritten... :)

Answer 4

Originally designed in Visual Basic in 2003 or earlier, no major design changes since

That alone is reason enough for a rewrite.

Answer 5

In my experience, developers always want to rewrite (self included).

The important thing is to take a step back and ask yourself what role the code is going to play in the company.

1. Is it going to require a lot of changes over time?
2. Do those changes interact with the design flaws (dependencies) you outlined earlier?

If yes to both, then do a rewrite. If it is a fairly self contained piece of code that does not require a lot of changes, leave it be.

Answer 6

I think the problem is to convince management that the code should be rewritten... after all it works. If a car goes and takes me from point a to b why should i change it. In other hand, just 2 or 3 years ago business men were asking themselves if youtube or facebook were a profitable business...

Answer 7

Your scenario sounds very familiar. It's the same discrepancy as everywhere between management and development.

It's not bad enough for a rewrite, because it's still working and customers seem to be content with the application. So why change a running system? This was your company's point of view.

It's bad enough for a rewrite, because it doesn't conform to current coding standards and practices, it's not well designed, hard to maintain and develop. This was the programmers point of view.

At last, it's the company's decision, which way to take. I had a similar situation, and then the company decided to continue with the old product, but to also raise a newly build system which would be introduced for new customers (if suited).

Answer 8

Any of us can tell you that the code merits a rewrite. However, after that you have at least two more issues:

1. How do I convince management?
2. What's the best way to manage working on two codebases on the same time to ensure that maintenance fixes are also included in the rewrite?

The second question above implies that you will get management approval. Given that they've already declined to use a rewrite you've prototyped, approval for the rewrite seems questionable at best. An alternative strategy is to clean up code as you go along where possible. Over time, the code base will improve and your fixes will be faster.

In any case, getting unit tests set up should be your top priority.

Answer 9

By all means strive to improve the codebase little by little.

There's one thing I would like to add to what others have said: once you've broken functions/methods down into smaller (and sanely named) ones, write - however difficult that might be - quick unit tests here and there. Try to get into the habit and stick with it.

Every chunk of code for which you have a test can be seen as "some more safe ground" upon which you can walk with more confidence than before towards the next change.

At the company where I work, we're about to rewrite a big, very complex application which is now written in VB6 to bring it to the web.

Without going into too much detail, I can grant you I would rather be in your shoes: only two developers on the team, bugs to fix and implementations to be done in the current version and the management assigning idiotic tasks every few days.

Answer 10

Assemble a list of customer complaints. Track their time to resolution and the issues you had with the code base in resolving them. Use that as your driver for being assigned the time to handle the technical debt.

Beautiful code doesn't make your salary - paying customers do. Demonstrate how to increase the income velocity by creating beautiful code.

Answer 11

[adding another answer, since it's a different aspect than my previous answer]

Elsewhere you mention the customers are unhappy, and one wouldn't be a customer if he didn't have a contract. Others have mentioned rewrites never go smoothly, and will have bugs and/or missing features when delivered.

If you are going to do a rewrite, try to get the customer(s) involved as much as possible. Try to make them feel like they are part of the team making the new software. The more they feel like they are involved in it, the less they will criticize the final result. They will better understand that it won't be flawless from day one, and know where to take their bug reports.

You can use that to sell it to management as well; it will distract the customers, they see something is being done about their problems.