Follow Up: Legacy code maintenance and moving forward (implementing previous answers)

Question

Overview

This is a follow up to my previous question that was flagged as a duplicate, and I do not know what would work better as the title.

Here is a link to the previous question: Legacy code: what to do in regards to maintenance and moving forward?

And the link to the proposed duplicate: What is the most effective way to add functionality to unfamiliar, structurally unsound code?

Here are some brief points from the discussions and answers of the original question:

• Premise was to discuss maintaining and modifying legacy code
• the legacy code is in a way where many of the object are quite large and bridge heavily into the idea of God objects; Performing functionality on numerous entities and coupling them in a way that has proved difficult to add or define/pull out some use case
• The main arguments were being consistent with the legacy code ideas of an object vs. making something new (in terms of modifying existing code or new code)
• Team-wise there is no architecture, and the healthy discussions around the eventual goal are not happening
• The development process is more reactive than anything else, so the idea of dealing with legacy code as a team changes constantly
• From a sort of professionalism/management chain perspective, there was a lot of feedback simply because this area ended up being the overall root problem in my actual environment

I do want to give due diligence in discussing the software side of the problem, since that was the intended purpose of the way I tried wording the previous question. (The separation and wording is mainly myself being green and attempting to be vague, so that I am not talking specifics)

Main Discussion

Now addressing the duplicate in respect to my question, I do see where this is a valid duplicate, but I am unsure about how to implement the selected answer. The summary that I gather is, "For smaller changes, then stay consistent. For larger changes, take the opportunity to refactor the area affected by the request. Do not refactor without a reason that is in line with some request from a work item, user story, etc."

For my scenario, I get lost on how to implement/incorporate the answer when it comes to tightly coupled functionality that came about (deployed as production code) unintentionally or requirements previously may have had them tied together. Other than the code base, there is a lack in documentation on business rules and many of the previous requirements.

As for consistency or refactoring as a discussion point in the development process, how does that answer apply when the new/modified functionality changes or breaks existing functionality that is not part of the request?

To me, it feels like the scenario lends itself to something like the Open/Close Principle, but I do not know if the code base is in a state that can support the principle well enough (testing, documentation, etc.). So my idea of any takeaway discussion points, could be centered around the God object and defining if the thing being open/close is the God object itself or some grouping of functionality defined as logical objects.

My understanding of the principle - the previous functionality is Open until the new piece is made, then the previous functionality can/should be closed. (Not too sure if I am grasping the idea of that principle correctly)

Summary

I am trying to be careful with my wording, so please feel free to request/do an edit that makes more sense. I do have a sense that I am wording things like I want to ignore the management/leadership piece, and that could make the content unclear. For an answer, I am looking at this question from a development/developer/programmer perspective, so that I could get an understanding of the possible role(s) developers play in this scenario, in respect to weighing in their opinions/ideas.

As previously stated, my original question has already branched out on the management/leadership side to this scenario. This separation that I am pushing could stem from being green/new and not having a grasp on if these pieces can/cannot be separated.

Answer 1

With the assumption that your team and management are on board to untangle the mess of the application, the most pragmatic answer is:

Change something and see what breaks.... then roll back

There's a good book called The Mikado Method that goes in to a lot more detail than I have space for here. The inspiration is a game of pick-up sticks (also known as Mikado) where the goal is to get as many sticks without disturbing the rest of the pile as possible.

The process is summarized like this:

• Come up with your target goal. Pick something focused.
• Try to code toward that goal and see what breaks.
• Roll back and come up with a solution to the next problem you see
• Rinse and repeat until you have a list of tasks that help you resolve the initial goal

Of course the easy things that work without breaking anything else you commit. A process like this requires robust version control software. If you don't have version control software you trust, this is your first task. The ability to roll back or abandon a branch is critical. I would highly recommend Git or something equivalent for this purpose.

The bottom line is that complicated software with hidden interdependencies is not going to be easy to tease apart into a more structured and testable architecture. It's going to be the accumulation of small, reversible changes that will ultimately add up to a large change. But each change you attempt has to be toward a well-defined goal. I've worked with legacy code bases that don't have any logging, so the only way to really understand how things were working was to add or improve logging using a real logging library. Again, each step you take should help you solve the end goal you are trying to get to.

Answer 2

I like Ike's answer to your original question.

In short, you tackle the problem much like a mouse eating an elephant - you take one bite at a time, and accept it's going to be a very long process.

For example, you might choose to select one of the many god objects and carve out a demi-god. Your new code will be just as messy and scary as all the existing code, but you (may have) made a small dent in the problem by isolating one tiny bit.

Think of it as taking a Top Down approach to the problem.

Repeat a zillion times and test like mad.

Or, as an alternative (as Ike suggested), find a new job and let this be someone else's problem.

Answer 3

Consistency is nonsense as I see it in this context. Walking in the wrong direction for the sake of consistency with legacy practices is still moving backwards. I'm assuming the very engineering practices that are now being favored for "consistency" are the very same practices that got you all into the mess, and that it is indeed a mess which results in problems and delays, and that this air of unreliability you discussed earlier is indeed resulting in developers unable to make changes without frequently running into unanticipated side effects and users running into bugs.

That said, it does help as a team to consistently choose a new direction as Berin points out. That could mean everything from revising and rethinking the coding standard, devising ways to test the software, agree on common refactoring strategies for the most recurring problems, work closely together on very concrete user-end goals, etc. If you can manage to sort that out as a team, that's the kind of consistency you need moving forward, not consistency with respect to the old practices which will effectively move you backward. Adding more methods or, worse, additional state to maintain to a monolithic God object in an unreliable codebase just for the sake of "consistency" is seriously moving backwards.

That said, if I was to pick a refactoring strategy beside the TDD-oriented approach of Michael Feathers towards every part of the system that might be refactored for whatever reason, it would be one that tries to establish clean designs and interfaces as soon as possible without a heavy iterative process. The method Berin outlined is one that I tried a lot personally with hundreds of Mercurial branches where I attempted a central change that was desperately needed to faulty areas, only to find over 100k lines of code breaking with something as simple as removing or replacing a single function. That said, I did it in the smallest ways, not trying to work towards a clean design so much as fix an immediate design problem which developers repeatedly tripped over which was repeatedly incurring many bugs (as a simple example there was a function which wasn't even reentrant that modified globals that some developers were trying to call in a multithreaded context -- I tried to make a basic change to make it reentrant by no longer making it modify globals and instead return results by value only to find that this basic changes broke over 100k lines of code in the system). I didn't have a clear end goal in sight beyond fixing this immediate problem either at the user or engineering level, and further I was hindered by the lack of support by the rest of the team.

Personally, in such a situation, the solution that gave me the most peace of mind and understanding of how to solve the conceptual problem of how to implement a new feature or improve an existing one was to wrap the old code like so:

class NewType
{
public:
    /// @return Access to the old type and old interface.
    /// This is to be removed when there are no longer
    /// dependencies to the old interface as they gradually
    /// get weeded out, however long that takes.
    OldType& old();

    /// Design a new interface here that's clean and ideally
    /// one you test against. You can use the old type to
    /// implement the interface, but don't refer to it too
    /// much to design the interface. Focus on the high-level
    /// conceptual requirements, perhaps only a subset of the
    /// requirements 'OldType' fulfills if its monolithic, and
    /// come up with the cleanest, most well-documented,
    /// well-tested interface you can fitting whatever square
    /// pegs through round holes you need in the implementation
    /// temporarily to iniitially implement this clean design
    /// agains the old type.

private:
    OldType old_type;
};

However, you don't want to think of it too directly like a basic "wrapper" which might tempt you to design the new interface too similarly to the old. You're really using that blank canvas that NewType provides to design a wholly new interface that's well-thought out in advance for the subset of the fundamental requirements that OldType fulfills. Besides the old() method to expose a handle to the OldType to avoid breaking everything all at once, this should be a whole new approach to rethinking the interface design and the new interface might only perform a subset of what OldType fulfilled (what the new type shouldn't do is just as important to think about as what it should do).

Now everything relying on the old type/interface has ugly-looking code like this if it isn't just being passed OldType as a parameter:

new_type.old().old_function();

That old part is really ugly, but if you are allowed to do it without making the rest of your team hate you, then it can be a useful transition step for refactoring, also making it blatantly clear which parts of the system depend on old interfaces to be gradually weeded out, and strongly discourage developers from adding new code which further adds more dependencies to the old interface. Unlike a highly iterative approach where you try to gradually arrive towards a saner design, you arrive at a clean design that's easy to test right away and write new client code against that is clear and easy to reason about. The implementation might still be fugly as hell to get the new interface to work and be implemented using the old type and interface, but at least you have a clean, new interface for a start to write new code and test against and gradually use to replace code using the old interface.

As for consistency or refactoring as a discussion point in the development process, how does that answer apply when the new/modified functionality changes or breaks existing functionality that is not part of the request?

With a tightly coupled codebase with many interdependencies, any kind of change could get you seemingly lost focusing on problems on the other side of the world. You could be working on particle systems initially only to end up, through a tangle of breakages and bugs, find yourself spending all night concentrating on how a string library and text-based search engine should ideally work because the changes to the particle system breaks breaks the scene graph which depends on the particle system which breaks a GUI dialog which depends on the scene graph which breaks the string library which depends on the GUI dialog (WTF?) which then breaks the search engine which depends on the string library.

It's why I think you have to start out just hoisting out a clean interface and use that as much as possible for new code and seek to, very slowly and gradually, make old code use the new interface if you're going to keep it. Eventually you might no longer find a need for it if you keep moving forward.