Baby steps

I’m in the middle of a massive refactoring and its going quite painfully.  It’s ending up as a massive change that hits a large number of files in our code base.  The issue is that for a while we were passing around raw BYTE*’s around and I’m trying to instead use a proper object model because of all the issues we’ve had with this.  The reasons we had this originally are somewhat historical and relate to how metadata is read out into byte streams with the IMetadataImport API.  However, it’s really been far more hassle than it’s worth.  Especially with the advent of generics and the need to store strongly typed structured data. 

Unfortunately, I just couldn’t figure out a simple way to do this change.  I was introducing an entirely new object model into a system that had none, and I was replacing raw reads and writes on the byte stream into safe calls into those objects instead.  I was also moving a lot of unnecessary static helper methods scattered all over the place into appropriate virtual methods on the new types.

I think I was biting off too much to try to chew pleasantly.  But I’ve done a ton of the work now, and I’m stuck in a hard place.  Start over again trying to make smaller steps (which I’m not even sure how to do).  Or continue with this huge change and end up fixing lots and lots of stuff because of all the regressions that inevitably happen after a change this large.

Comments (4)

  1. Mark says:

    This may not work with your situation but in at least a couple of refactoring situations I have had to do that sound similar this technique has proven helpful:

    Instead of trying to introduce the object model right off from the start I have created a temporary singleton class that also acts as a facade for my new object model. Initially it wraps much of the kruft that is getting refactored away. It also provides a a global (yuck) repository for ‘stuff’ that needs to be put someplace during the refactoring. The old code is refactored to use the singleton class.

    However, the purpose of this ‘kruft can’ singleton is temporary. The singleton is refactored bit by bit to internally use the new object model and then the object model is ‘leaked’ out into the rest of the code. By the end of the process the singleton is gone and my nice new object model is there. But by using the singleton as a ‘global’ facade for the new object model it allows the refactoring to progress in smaller steps than it might have otherwise.

    I have also seen situations where this would not work very well so take it for what it is work.

  2. jaybaz [MS] says:

    I’d approach it like this:

    1. Wrap the BYTE* in a class. Leave it public. Change all the code from passing the BYTE* to passing the new class. You’ve now made your methods more type-safe & their intent is clearer. Run tests, check in, take a break.

    2. Pick a usage of the BYTE*. Create a method in the new class that does what you need (ExtractMethod, then MoveMethod). Replace the direct usage with the method call. Repeat until you get to a good stopping point. Run tests, check in, take a break.

    3. When there are no more direct users of the BYTE*, change the BYTE* to be private. Run tests, check in, and celebrate. You have now encapsulated the byte stream manipulation.

    4. As needed, refactor this new class to make sense.

  3. Jay: You could have just said "What Mark said" 😉

    I agree. This would have been a much safer way of making this change.

    Thanks to both of you for the advice.