20

I've been asked to evaluate what appears to be a substantial legacy codebase, as a precursor to taking a contract maintaining that codebase.

This isn't the first time I've been in this situation. In the present instance, the code is for a reasonably high-profile and fairly high-load multiplayer gaming site, supporting at least several thousand players online at once. As many such sites are, this one is a mix of front- and back-end technologies.

The site structure as seen from the inside out, is a mess. There are folders suffixed "_OLD" and "_DELETE" lying all over the place. Many of the folders appear to serve no purpose, or have very cryptic names. There could be any number of old, unused scripts lying around even in legitimate-looking folders. Not only that, but there are undoubtedly many defunct code sections even in otherwise-operational scripts (a far less pressing concern).

This is a handover from the incumbent maintainers, back to the original developers/maintainers of the site. As is understandably typical in these sorts of scenarios, the incumbent wants nothing to do with the handover other than what is contractually and legally required of them to push it off to the newly-elected maintainer. So extracting information on the existing site structure out of the incumbent is simply out of the question.

The only approach that comes to mind to get into the codebase is to start at the site root and slowly but surely navigate through linked scripts... and there are likely hundreds in use, and hundreds more that are not. Given that a substantial portion of the site is in Flash, this is even less straightforward since, particularly in older Flash applications, links to other scripts may be embedded in binaries (.FLAs) rather than in text files (.AS/ActionScript).

So I am wondering if anyone has better suggestions as to how to approach evaluating the codebase as a whole for maintainability. It would be wonderful if there were some way to look at a graph of access frequency to files on the webserver's OS (to which I have access), as this might offer some insight into which files are most critical, even though it wouldn't be able to eliminate those files that are never used (since some files could be used just once a year).

5
  • 7
    I don't know enough about flash but if you get compilation errors when code isn't there, you should be able to rename folders to see if they are referenced.
    – Oded
    Jan 26, 2012 at 15:58
  • Evil solution: Delete them and wait for the errors/bug reports. (Just make sure it's recoverable!)
    – Izkata
    Jan 26, 2012 at 16:20
  • 1
    @Nick Could you clarify if you're being paid for the evaluation as part of the next phase of contract that you still have to bid on/otherwise get? Your answer won't change the "is there a tool" question, but some of us could craft answers re: process that would be better suited to your situation (e.g. keep you from getting screwed, etc).
    – jcmeloni
    Jan 26, 2012 at 16:32
  • @jcmeloni No, I'm not being paid for the evaluation. But in my experience, and from small things I have picked up in the last couple of days, they don't have anyone else at the table right now. My skillset is fairly unusual, so I'm even more at ease that they don't have anyone else competing for it, based on the quote. The actual quote in question is from my client-to-be to their client, who is planning to re-award them the contract. Really from my end, I am meant to assist them in providing said quote. HTH.
    – Engineer
    Jan 26, 2012 at 16:40
  • @Oded Rename is definitely easier than trial-and-error deletion! Good thinking there. That's one more tool in the box.
    – Engineer
    Jan 26, 2012 at 16:49

7 Answers 7

31
+50

Since what you're being asked to do is provide input for your client to write an appropriate proposal to the other client (owner-of-the-nightmare-code) for any work on that code, I'm going to go out on a limb and say that you're not going to be doing any thorough testing or refactoring or anything along those lines at this point. You probably have a very short time to get a rough estimate. My answer is based on my experience in the same situation, and so if my interpretation is incorrect, just disregard everything that follows.

  • Use a spidering tool to get a sense of what pages are there, and what is inbound. Even a basic linkchecker tool -- not a specific "spider for auditing purposes" tool -- will be useful in this regard.
  • Make a basic audit/inventory spreadsheet. This could be as simple as a list of files and their last-modified time, organized by directory. This will help you get a sense of scope, and when you get to directories like _OLD and _DELETE you can make a big note that a) your evaluation is based on stuff not in those directories b) the presence of those directories and the potential for cruft/hidden nightmares attests to deeper issues that should be accounted for in your client's bid, in some way. You don't have to spend a gazillion years enumerating the possible issues in _OLD or _DELETE; the info will feed into the eventual bid.
  • Given you are reviewing what sounds like an entirely web-based app, even standard log analyzer tools are going to be your friend. You will be able to add to the spreadsheet some sense of "this is in the top 10 of accessed scripts" or some such. Even if the scripts are embedded in Flash files and therefore not spiderable, there's a high probability they are accessed via POST or GET, and will show up in the server logs. If you know you have 10 highly accessed scripts, not 100 (or vice versa), this will give you a good idea of how maintenance work will likely go.

Even in a complicated site, what I outlined above is something you could do in a day or day and a half. Since the answer you're going to give to your client is something like "this is going to be a tremendous pain in the butt, and here are some reasons why you'll just be putting lipstick on a pig, so you should bid accordingly" or "any reasonable person would bid not to maintain but to start over, so you should bid accordingly" or even "this isn't that bad, but it will be a consistent stream of work over any given timeframe, so bid accordingly", the point is that they're going to be making the bid and thus you do not need to be as precise as you would be if you were being hired directly to do a full content and architecture audit.

2
  • 2
    +1 This is a fantastic answer. Where's that +5 button gotten to...
    – Engineer
    Jan 26, 2012 at 17:23
  • 1
    TL;DR: don't send yourself down a rabbit hole until you have to. :)
    – jcmeloni
    Jan 26, 2012 at 17:45
4

I strongly recommend refactoring the existing source code (as opposed to a rewrite) using the patterns found in the book "Working Effectively With Legacy Code".

The book details several mechanisms for efficiently covering legacy code in unit tests, so that you can then begin to safely refactor the code. The book is broken in parts, one describing the philosophy behind the approach, and then several chapters that solve particular problems, such as "It takes forever to make a change", "I don't have much time and need to change it", and "I can't get this class in to a test harness". Each of these chapters has detailed, proven techniques that help you learn how to apply best practices in testing to real world problems.

Reading the book left me with a very real sense that "we are not alone" ... many of us, or perhaps all of us, are working with complex code bases that have become difficult to manage. The techniques listed in the book have given me much hope, and I've personally been able to apply them almost immediately.

Joel Spolsky's blog post does a great job of explaining why its best to keep an existing, working code base as opposed to starting from scratch. I've chosen a quote from the article that sums it up, but its a fantastic read.

"There's a subtle reason that programmers always want to throw away the code and start over. The reason is that they think the old code is a mess. And here is the interesting observation: they are probably wrong. The reason that they think the old code is a mess is because of a cardinal, fundamental law of programming:

It’s harder to read code than to write it.". - http://www.joelonsoftware.com/articles/fog0000000069.html

1
  • 5
    +1. In response to Joel's comment, "It bloody well shouldn't be." Because I don't see the problem as inherent. I see it as being partly the fact that many people write shoddy code and don't care, while many others write reasonably good code but live by the "self-documenting code" concept... which is just plain BS: One may flatter one's own coding style all one wishes in privacy, but when it comes to public codebases just spawn comments like there's no tomorrow. Doesn't hurt. And finally there are people who have to get things working in a legacy codebases, on a tight time budget.
    – Engineer
    Jan 26, 2012 at 17:10
2

In a typical Java code base, I'll consider using tools such as PMD, FindBugs, or Sonar and then I'll try to understand tools reporting (dead code, undocumented code, duplicated code, etc.)

Based on the reports I'll try to find the different layers of the application/site (business layer, DB, SQL, etc.)

If layers are coupled (html within servlet, sql within java code) I'll start first by decoupling each of these steps should be considered to be isolated and you may commit at the end of each one (by starting a branch then make merge).

1
  • 1
    Thanks. Although your answer's somewhat Java specific, it's interesting to see your layered approach... peeling the onion, so to speak. Something to think about.
    – Engineer
    Jan 26, 2012 at 16:44
1

From your description it seems that this code has hit the unmaintainable state, which means the the best approach is likely a complete rewrite. Developers would have a lot smaller paychecks if there were quality tools that worked to keep a messy code-base maintainable. It is possible to go through and clean up the old unneeded code from folders, but its a manual task and you likely won't get everything anyway without unreasonable amounts of time. I'm just guessing here, but I bet the working code itself is just as much of a mess as the file structure which means even when you manage to get the code-base trimmed to the actively working code its still going to be a nightmare to update or fix anything.

I would stress that the effort required to get the existing code in a maintainable state would be equal or greater than the effort to start over on a rewrite. part of maintaining anything is knowing when to "take it behind the shed and shoot it".

4
  • Ordinarily I'd be 100% with you on the toss-and-rewrite approach. But in this instance (and at least for now), I am to be paid just for work to maintain the site, rather than a more extensive overhaul which would take several weeks. Also, even if I wanted to right now, I couldn't keep up with doing that and holding down the other contracts I have on the go, since my weekly availability for this is explicitly limited -- my primary contract must be fulfilled to its 40 hour weekly minimum.
    – Engineer
    Jan 26, 2012 at 16:36
  • 1
    Disagree with toss and rewrite! From joelonsoftware.com/articles/fog0000000069.html ... "There's a subtle reason that programmers always want to throw away the code and start over. The reason is that they think the old code is a mess. And here is the interesting observation: they are probably wrong. The reason that they think the old code is a mess is because of a cardinal, fundamental law of programming: It’s harder to read code than to write it." Instead, I strongly recommend refactoring: amazon.ca/Working-Effectively-Legacy-Michael-Feathers/dp/…
    – Kyle
    Jan 26, 2012 at 16:56
  • 2
    @KyleHodgson sometimes the code actually is a mess, and when you are at the point that its a mess to find the code before reading it, its time to start over.
    – Ryathal
    Jan 26, 2012 at 18:15
  • Yeah, I don't think it's as clear cut as that, though that book looks worth reading. It does depend very much on the size/complexity of the codebase, and warm bodies available to do the work.
    – Engineer
    Jan 26, 2012 at 19:40
1

A web crawler might help you determine which URLs are accessible. Especially if it's smart enough to extract links from Flash or JavaScript. Once you have a list of web pages, go through them and list the files they refer to. Anything that's left over after this process should be considered dead code.

1
  • 1
    I strongly disagree with your last sentence. Crawler can only find out which pages are linked together as a directed graph with a single or multiple starting points. But as we speak of a website, there are also so called "landing pages", which link to other pages but there is no links pointing to them. Also, there might be old parts of administrative interface which are also disconnected from other pages. I currently have a project of this type.
    – scriptin
    Dec 31, 2012 at 13:53
0

Note: I put an accent on the database usage, while you asked about the usage of the code itself. The answer still applies to both cases in every point I mentioned.

You already answered in part your own question in the last paragraph: see what is accessed while the application is running.

  1. You may want to profile the database and to ask the profiler to record all the queries for a day. It will give you an overview of the most used database objects, but will not tell which ones are never used. Also, you must still be careful with the results: for example a table may be used exclusively through stored procedures, but when you'll look at the queries from the profiler, it would appear as if the table is not used at all.

  2. Reviewing the source code, searching for queries is more helpful, and after collecting all queries, you can have a good understanding of database usage, not in terms of frequency (this is where a profiler is handy), but in terms of used/not used tables. Sadly, for a badly written/not maintained for years codebase, it may be extremely hard and error prone, especially if queries are constructed dynamically (imagine a method which, in a select, uses a parameter as the name of the table; how can you possibly know what are the possible values of the parameter by just looking at the source code?).

  3. Static analysis and some compilers may also reveal dead code, but still doesn't give you the answer you want.

  4. Analysis of data itself or the database metadata can reveal some interesting info. For example, it would be easy to assert that the table LogonAudit(uniqueidentifier LogonAuditId, datetime LogonEvent, ...) is not used any longer if it contains 10 000 records per day for the years 2006 to 2009, and no records from September, 18th, 2009. The same is not true for a table which contain the data indented to be mostly read-only.

Those four points together will give you the list of used tables. Remaining ones are either used or not. You may make assertions, and test them, but without good unit tests coverage, it wouldn't be easy. Any "easy" way would fail too. For example, if you have a products_delme_not_used table, you may assert that the table is not used at all, and check for "products_delme_not_used" in your code. This is optimistic: it's not unusual to find the DailyWTF candidate like this in an old codebase:

// Warning: WTF code below. Read with caution, never reuse it, and don't trust
// the comments.

private IEnumerable<Product> GetProducts()
{
    // Get all the products.
    return this.GetEntities<Product>("PRODUCT");
}

private IEnumerable<T> GetEntities<T>(string tableName)
{
    // Everyone knows that SQL is case sensitive.
    tableName = tableName.ToLower();

    if (tableName == "user" || tableName == "product")
    {
        // Those tables were renamed recently in the database. Don't have time
        // to refactor the code to change the names everywhere.
        // TODO: refactor the code and remove this `if` block.
        tableName += "s";
    }

    if (this.IsDelme(tableName))
    {
        // We have some tables which are marked for deletion but are still
        // used, so we adjust their name.
        tableName = this.Delme(tableName);
    }

    return this.DoSelectQuery<T>("select top 200 * from " + tableName);
}

private bool IsDelme(string name)
{
    // Find if the table is among candidates for removal.
    List<string> names = this.Query<string>("select Names from DelmeTables");
    return names.Contains(name);
}

private string Delme(string name)
{
    // Return the new name for a table renamed for deletion.
    return string.Join("_", new [] { name, "delme", "not", "used" });
}

Can you figure out that this code actually uses products_delme_not_used table?

If I were you, I would:

  1. Keep all the database objects in place,
  2. Refactor the whole application (if it's worth it),
  3. Document (while refactoring) the application and specifically the database usage.

When you finish the last two steps, you'll probably have a better understanding of the database usage, which will help figuring the names of the tables which are not used any longer, and may more or less safely remove them.

0

It sounds to me you need to get enough information for creating a quote so I'll concentrate on that effort.

I would try to determine how many use cases there are involved in this site. This typically gives you an idea of how big and complicated the site is and how much time it will take to re-create or maintain the site/application.

Yes, it's true that sometimes code isn't used any longer and it will make the application look a little bit bigger than it really is, but I don't think this is going to affect the numbers by more than 20% at the most, so I wouldn't worry about that part.

Looking at the source code, web pages and database tables should help you discover this.

You may also want to consider limiting the number of hours per month that you'll spend on this project for the pre-determined fee to protect yourself.

As far as discovering what is being used and not used, there really is no easy way. Code analysis tools may help, but since you're dealing with such a mixed bad I don't think any single tool exists that can help. For each specific area you can probably find a code analysis tool that may help.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.