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
- External documentation is just an overview. I've written decent documentation since then.