3

I've been working in an application with about 100k LOCs and basically We have been reworking features to comply with a new architecture based on a new technology stack. This work is almost finished but we had to keep the old code because we had to guarantee a stable environment in case of critical bugs in the new .

This has lead to a situation where we have a lot of dead code that mixes with a lot of working code thus making difficult to estimate how much time it would take to remove this dead code.

This codebase in particular is very troublesome as it does not have unit/integration testing other than the one generated in the process of reworking the features we migrated to the new architecture and also I think that a great deal of the new code is interwoven with the dead code (by static calls, using the previously thrown exceptions).

What would be a good start point to estimate how long would it take to safely remove the code?

Improve this question
6
  • 2
    Do you have a static code analysis tool that can identify the dead code?
    – Robert Harvey
    Commented Dec 23, 2013 at 17:44
  • 1
    Think: "It couldn't possibly take longer than ______ to get rid of that stuff!" Got it? What's the blank? Cool, now multiply by 3. The nuances of old codebases - particularly ones that grew organically are always troublesome and killing them always proves more difficult than anticipated.
    – Telastyn
    Commented Dec 23, 2013 at 17:57
  • I'd like to point out that static calls are a highly inappropriate way to add new code or architectures to a legacy operating environment. If you have any static coupling of new code to legacy code (or legacy data, for that matter) then you're not really improving matters at all. If it's a website, you should be able to just change a URL. And if it's a desktop, mobile, or other smart client, then it's not a good candidate in the first place for introducing a new technology stack without a complete rewrite - you need to make slow, incremental changes instead.
    – Aaronaught
    Commented Dec 23, 2013 at 20:53
  • Well, it would have been a good idea to mark every method beforehand when you were writing a replacement for it, to make it easy to remove it afterwards. Now it is a little late for that, but maybe not too late. Start adding comments with a searchable tag to all the dead functions. Then count the comments, this gives this will give you a rough feeling how much work you will have to invest.
    – Doc Brown
    Commented Dec 24, 2013 at 6:08
  • 1
    Create a copy of the project, rename all the old/dead stuff (e.g. append a "_old" prefix to names), then see how many "undefined class/method/whatever" errors you get when you try to compile.
    – Brendan
    Commented Dec 24, 2013 at 6:16

3 Answers 3

Reset to default
3

Each project is going to be different so there is not an effective way to estimate the time to remove dead code. Needing to keep the old code sounds like you lack a quality source control system that allows you to branch.

In any case: Tools like SonarQube can help identify some dead code, but won't find things like a class that is never used. I would suggest tackling a portion of the code by hand with the help of SonarQube and develop estimates as you go. If you work through 20% of the classes and it takes 2 weeks then you can estimate an additional 8 more weeks to do the rest.

Also, having some sort of automated testing will help prevent you find code that is mistakenly deleted faster.

Improve this answer
7
  • 1
    Classes that are never used are easy to detect in an IDE (one at a time). Seems like an odd omission.
    – Robert Harvey
    Commented Dec 23, 2013 at 17:54
  • There might be tool that exists or a rule that you can add to find these classes, but in my experience using and IDE one at a time is the only way I have found. When you are talking 100k LOCs that sounds painful and worth investigating further.
    – jzd
    Commented Dec 23, 2013 at 17:56
  • 4
    @RobertHarvey: It's not really that simple. Objects like controllers, views, and areas are fundamental to an MVC architecture but are almost never directly referenced by code, so they would appear "unused" to a static analysis tool. Even many DI frameworks like Castle Windsor now have very good support for automatic convention-based registration, so you can have all kinds of classes that are never directly referenced anywhere except through an interface that they may or may not be the canonical implementation of. You need dynamic analysis for this, not static analysis or IDE support.
    – Aaronaught
    Commented Dec 23, 2013 at 20:44
  • 1
    @Aaronaught: Ah, yes. If memory serves, controller methods in ASP.NET MVC are executed using Reflection.
    – Robert Harvey
    Commented Dec 23, 2013 at 21:54
  • 1
    @RobertHarvey: It's a little more complicated than that in practice - you've got dynamic dispatch, expression tree compilation, IL emission and more. But if it helps to think of it all as reflection, that's fine - the important thing is that it's not statically linked to the program's entry point, and reflection does get involved at some point.
    – Aaronaught
    Commented Dec 24, 2013 at 1:04
6

How much is your company worth?

No, seriously. How much?

Because Knight Capital failed to practice good housekeeping with their systems. And that led to a glitch that ended with their being acquired at firesale prices.

Either the code is dead, or it isn't. If it is dead, then get rid of it. If it isn't, you need to know what's calling the code and make the necessary changes so you can kill off the old code.

If you don't know for sure that it's dead, then either profile your code or make it throw obnoxious, impossible-to-miss errors when those presumed-to-be-dead code paths are hit.

Estimating time / cost for that removal depends upon how well structured the code is to begin with. If the new code is well isolated from the old code, then it should be a minimal cost to change out. If you haven't isolated your code, then it's hard to predict how tangled things are. The more tangled it is, the higher your costs and time.

Improve this answer
2
  • KC's issue wasn't just due to dead code - they actually tried to reuse legacy data/configuration for an alternate purpose, which is a no-no in complex systems. I agree with the "impossible-to-miss errors" part, though; it's essentially just an application of the Fail Fast doctrine.
    – Aaronaught
    Commented Dec 23, 2013 at 20:48
  • 2
    That's pretty much what happened to the Ariane 5.
    – Robert Harvey
    Commented Dec 23, 2013 at 21:54
0

What if you using a global find and replace entered this into the constructor of every class: 

PrintToMyLogFile(this.className);

Run your higher level tests including manual tests. Then delete all of the files that arn't in your log file.

This obviously is going to break some things but you can fix those as you go along. This will ensure that you will be able to remove all classes that are not used.

In reality this probably isn't a great way to go about it and probably has multiple issues that I haven't considered yet.

Improve this answer

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.