How to prevent unknowningly duplicating code

clean codedesign-patternsrefactoring

I work on a rather large code base. Hundreds of classes, tons of different files, lots of functionality, takes more than 15 minutes to pull down a fresh copy, etc.

A big problem with such a large code base is that it has quite a few utility methods and such that do the same thing, or has code that doesn't use these utility methods when it could. And also the utility methods aren't just all in one class (because it'd be a huge jumbled mess).

I'm rather new to the code base, but the team lead who's been working on it for years appears to have the same problem. It leads to a lot of code and work duplication, and as such, when something breaks, it's usually broken in 4 copies of basically the same code

How can we curb this pattern? As with most large projects, not all code is documented(though some is) and not all code is… well, clean. But basically, it'd be really nice if we could work on improving the quality in this respect so that in the future we had less code duplication, and things like utility functions were easier to discover.

Also, the utility functions are usually either in some static helper class, in some non-static helper class that works on a single object, or is a static method on the class which it mainly "helps" with.

I had one experiment in adding utility functions as Extension methods(I didn't need any internals of the class, and it definitely was only required in very specific scenarios). This had the effect of preventing cluttering up the primary class and such, but it's not really anymore discoverable unless you already know about it

Best Answer

The simple answer is that you really can't prevent code duplication. You can however "fix it" through a difficult continuous repetitive incremental process that boils down into two steps:

Step 1. Start writing tests on legacy code (preferably using a testing framework)

Step 2. Rewrite/refactor the code that is duplicated using what you've learnt from the tests

You can use static analysis tools to detect duplicated code and for C# there are loads of tools that can do this for you:

Tools like this will help you find points in code that does similar things. Continue to write tests to determine that they really do; use the same tests to make the duplicate code simpler to use. This "refactoring" can be done in multiple ways and you can use this list to determine the correct one:

Furthermore there is also a whole book about this topic by Michael C. Feathers, Working Effectively with Legacy Code. It goes in depth different strategies you can take to change the code to the better. He has a "legacy code change algorithm" which is not far off from the two step process above:

  1. Identify change points
  2. Find test points
  3. Break dependencies
  4. Write tests
  5. Make changes and refactor

The book is a good read if you're dealing with brown-field development, i.e. legacy code that needs to change.

In this case

In the OP's case I can imagine the untestable code is caused by a honey pot for "utility methods and tricks" that take several forms:

  • static methods
  • use of static resources
  • singleton classes
  • magic values

Take note that there is nothing wrong with these, but on the other hand they're usually hard to maintain and change. Extensions methods in .NET are static methods, but are also relatively easy to test.

Before you go through with the refactorings though, talk with your team about it. They need to be kept on the same page as you before you proceed with anything. This is because if you're refactoring something then chances are high you'll be causing merge conflicts. So before reworking something, investigate it, tell to your team to work on those code points with caution for a while until you're done.

Since the OP is new to the code there are some other things to do before you should do anything:

  • Take time to learn from the codebase, i.e. break "everything", test "everything", revert.
  • Ask someone from the team to review your code before committing. ;-)

Good luck!