169

I started a new job recently where I am working on a very large application (15M loc). In my previous job we had a similarly large application but (for better or for worse) we used OSGi, which meant the application was broken down into lots of microservices that could be independently changed, compiled, and deployed. The new application is just one large code base, with maybe a couple of .dlls.

So I need to change the interface of this class, because that's what my boss asked me to do. They initially wrote it with some assumptions that didn't generalize too well, and for a while they have been avoiding the problem of refactoring because it's so tightly coupled. I changed the interface and now there are over 25000 errors. Some of the errors are in classes with important sounding names like "XYZPriceCalculator" which reaaally should not break. But I can't start up the application to check if it's working until all the errors are resolved. And many of the unit tests either directly reference that interface, or are coupled to base classes which reference that interface, so just fixing those is a pretty huge task in itself. Plus, I don't really know how all these pieces fit together, so even if I could get it to start, I don't really know what it would look like if things were broken.

I never really faced a problem like this at my last job. What do I do?

Improve this question
24
  • 7
    You're going to need to give us more information about the kinds of errors and what "this class" is, we're not mind readers.
    – whatsisname
    Nov 4, 2016 at 2:28
  • 140
    "over 25000 errors" such numbers shown in VS are often wrong. There are a few errors, but with the build of an often used dll broken, other builds do fail also, giving raise to astronomic error numbers. I always start fixing with the first error message found in the build output, not in the error list.
    – Bernhard Hiller
    Nov 4, 2016 at 8:41
  • 13
    I would like to see the before and after signatures on the method you changed. BTW - 25k errors doesn't really sound like too many to deal with. Tedious yes, daunting, yes, unmanageable, no.
    – jmoreno
    Nov 4, 2016 at 10:49
  • 47
    A public interface is a contract. Don't break such contracts -- create new ones.
    – Matthew Read
    Nov 4, 2016 at 15:09
  • 6
    25000 errors!? Houston, we got a problem. Definitely revert the change and talk to your manager, explain to him that you might have to create a new interface altogether.
    – Code Whisperer
    Nov 4, 2016 at 18:53

9 Answers 9

Reset to default
350

25000 errors basically means "don't touch that". Change it back. Create a new class that has the desired interface and slowly move the consumers of the class to the new one. Depending on the language, you can mark the old class as deprecated, which may cause all sorts of compiler warnings, but won't actually break your build.

Unfortunately these things happen in older code bases. There's not much you can do about it except slowly make things better. When you create the new classes, make sure you properly test them and create them using SOLID principles so they will be easier to change in the future.

Improve this answer
17
  • 80
    It does not necessarily have to be a new class. Depending on language and circumstances it might be a function, interface, trait, etc.
    – Jan Hudec
    Nov 4, 2016 at 7:23
  • 33
    It also helps if the old interface can be replaced by compatibility wrapper around the new one.
    – Jan Hudec
    Nov 4, 2016 at 7:24
  • 82
    Sometimes, 25000 errors actually means We never dared touching that, but now that a newcomer has arrived, let's give him that cleaning-out-Augean-stables task.
    – mouviciel
    Nov 4, 2016 at 10:46
  • 14
    @theDmi: I agree, 1 change and 25k errors most likely means 2.5k changes at most, and I wouldn't be surprised to find that it was around 250. Lots of work either way, but doable.
    – jmoreno
    Nov 4, 2016 at 11:04
  • 34
    Those 25000 errors might all be fixable by a single change. If I break the base class of a huge heirarchy, every one of the derived classes will spew errors about invalid base, every use of those classes will spew errors about the class not existing, etc. Imagine it was "getName", and I added an argument. The override in the "HasAName" implementation class now doesn't work (error), and everything inheriting from it now generates errors (all 1000 classes), as well as every time I create an instance of any of them (24x per class on average). The fix is ... one line.
    – Yakk
    Nov 4, 2016 at 13:42
80

Divide and conquer with refactorings

Often, breaking up the change that you need to do into smaller steps can help because you can then perform most of the smaller steps in a way that doesn't break the software at all. Refactoring tools help a lot with such tasks.

Divide

First, identify the smallest possible changes (in terms of logical changes, not in terms of changed LoC) that sum up to the change you want to achieve. Especially try to isolate steps that are pure refactorings and can be carried out by tools.

Conquer

For complicated cases like yours, it may make sense to perform one small refactoring at a time and then let the problem rest, so that all continuous integration systems can verify the change, and maybe the testing team had a look as well. This validates the steps you make.

To carry out a particular refactoring, you absolutely need tool support for a case where you have 25,000 call sites of the method you want to change. Maybe search-and-replace works, too, but for such a critical case, I'd be scared by that.

Example

In C#, for example, it's possible to use Resharper to change the signature of a method. If the change is simple enough, e.g. adding a new parameter, then you can specify which value should be used at call sites that would otherwise have a compile error.

You are then at an error free code base immediately and can run all unit tests, which will pass, because it was a refactoring only.

Once the signature of the method looks good, you can go and replace the values that Resharper added as arguments to the newly introduced parameter. This is not a refactoring anymore, but you have an error-free code base and can run the tests after each line you change.

Sometimes that doesn't work

This is a very useful approach for cases like yours, where you have a lot of call sites. And you do have working software now, so it may be possible to make small refactoring steps to change the signature just a bit, then do another one.

Unfortunately it won't work if the signature change is too complex and cannot be broken down into smaller changes. But this is rare; splitting the problem into smaller problems usually shows that it is possible.

Improve this answer
2
  • 13
    This. Refactor if you can (safely), otherwise you need a proper migration.
    – sleske
    Nov 4, 2016 at 7:46
  • 3
    And for the love of whatever, please make use of your IDE's built-in refactoring tools, rather than just changing the (e.g.) method signature locally and then going 'round fixing the consequent errors.
    – KlaymenDK
    Nov 7, 2016 at 13:14
36

Clarify your task with your boss to help him to understand the problem and your needs as a professional software developer.

If you are part of a team, look for the lead developer and ask him for advice.

Good luck.

Improve this answer
1
  • 17
    This is the first step I'd take - "I'm a junior and my boss told me to do a thing and now I'm getting a really big error message, but my boss didn't tell me about that." Step 1: "Hey boss, I did the thing, but I get 25k errors now. Is that supposed to happen, or..."
    – Pimgd
    Nov 4, 2016 at 10:22
29

Don't touch it. Don't commit anything.

Instead, sit down on your chair and shout "Heeeeelp!!!!!" as loud as you can.

Well, not exactly like that, but ask any of your senior colleagues for advice. If you have 25,000 errors, you don't fix the errors, you fix what caused the errors. And a senior colleague should be able to advice you how to make the change your boss wants without the 25,000 errors involved. There are various ways to do this, but what is a good way depends on your specific situation.

And it might be the boss told your senior colleagues to make that same change, and they said "no". Because they knew what would happen. That's why you were given the job.

Improve this answer
4
  • 2
    This is definitely an important thing to keep in mind. If a new employee is truly ambitious to a large degree, they might be industrious (gullible) enough to do the really big task that only ends up gaining a 5 byte memory advantage or be bit more logical to recode and maintain later.
    – user64742
    Nov 6, 2016 at 2:10
  • @TheGreatDuck: Tell me you've never seen an interface used everywhere and wrong everywhere. I sure have.
    – Joshua
    Nov 8, 2016 at 4:18
  • @Joshua that's not even relevant to what I just said. There are times were fixing a minor bug isn't always the smartest idea of it means rewriting 10,000+ lines of code. Unfortunately, a new employee might be gullible enough to pull 10 all-nighters outside of work rewriting that code to impress their boss and get it done. Sure, it's a great thing to do, but sometimes enough is enough. You just have to accept the bug's existence.
    – user64742
    Nov 8, 2016 at 14:17
  • 1
    @Joshua btw, I only joined this community because this question came in on my popular questions feed. I have no experience with large-scale design like this. I just agreed with this person's view.
    – user64742
    Nov 8, 2016 at 14:18
23

Entrenched APIs can't simply be changed. If you really need to change them, anootate and/or document them as deprecated (by whatever means the language allows) and document which API should be used instead. The old API can then be phased out slowly... perhaps very slowly depending on your time budget for refactoring.

Improve this answer
11

Evaluate

Evaluate whether this change is necessary, or whether you can add a new method and deprecate the other.

Forward

If changing is necessary; then a migration plan is necessary.

The first step is to introduce the new method and have the old method massage its arguments so it can call the new one. This may require hard-coding a few things; that's fine.

This is a commit point: check that all tests pass, commit, push.

Migrate

The busy work is migrating all callers of the old method to the new one. Thankfully it can be done gradually thanks to the forwarder.

So go ahead; don't hesitate to use tools to assist (sed being the most basic, there are others).

Mark the old method as deprecated (with a hint of switching to the new method); it will help you spotting whether you forgot anything and will help if a coworker introduces a call to the old method whilst you are working on this.

This is a commit point (or maybe several commit points): check that all tests pass, commit, push.

Remove

After some time has passed (maybe as little as a day), simply remove the old method.

Improve this answer
4
  • 5
    sed is probably a bad idea... Something that actually "understands" the language and won't make unintended drastic changes is better.
    – wizzwizz4
    Nov 5, 2016 at 20:02
  • 1
    @wizzwizz4: Unfortunately, I've found very few tools that actually understand the language well enough for C++; most tools seem to cater to renaming and that's it. Granted, renaming only the exact method calls (and not any overload or unrelated but similarly named method calls) is already impressive, but it's insufficient for anything more complicated. At the very least, you would need the abilities to (1) shuffle arguments, (2) apply a transformation to a given argument (call .c_str() for example) and introduce new arguments. sed kinda works, the compiler catches its issues afterward.
    – Matthieu M.
    Nov 6, 2016 at 12:07
  • 1
    sed (or ed) can be adequate for this sort of thing - as long as you properly review the diff before you commit.
    – Toby Speight
    Nov 7, 2016 at 16:19
  • This is the TCRR of html, yeah? :)
    – user248838
    Nov 7, 2016 at 23:38
8

If your change to the method signature is merely a name change, the simple solution is to use tools to automate the change in the 25,000 classes that reference the method in question.

I assume that you have simply edited the code manually, which gave rise to all the errors. I also assume you are familiar with Java (seeing your reference to OSGi), so for example in Eclipse (I don't know which programming environment you use, but other environments have similar refactoring tools) you can use "Refactoring -> Rename" to update all the references to the method, which should leave you with no errors.

In case you are making other changes to the method signature than simply renaming (changing the number or types of parameters), you can use "Refactoring -> Change method signature". However, chances are you will have to be more careful as the other answers suggest. Also, regardless of the type of change it may still be quite a task committing all those changes in a busy code base.

Improve this answer
3
  • 2
    OP specifically said OSGi was at their previous job, so it's not really relevant here.
    – user
    Nov 5, 2016 at 13:38
  • 3
    @MichaelKjörling If the OP was a Java programmer in their previous job, there's a better-than-evens chance they're a Java programmer in this job too.
    – Rich
    Nov 7, 2016 at 10:11
  • @MichaelKjörling I wanted to give a concrete recommendation, using Eclipse for refactoring because OP is familiar with Java. Whether OP is in fact using Java for the current project, I guess, is less important, but I should clarify my answer. Thanks.
    – MikkelRJ
    Nov 9, 2016 at 7:44
6

Here my contribution.

I started a new job recently where I am working on a very large application (15M lines of code).

You probably are not familiar with the project and its "features". Before to type down a single line of code, It's important to be familiar with the project. So do rollback your changes and start by analysing the code. (At least the affected one)

Understanding the existing solution gives you a better perspective of where are you getting into. Contextualise the solution and Its importance.

As @Greg pointed out, you should be able to test the existing code to have a valid reference to compare with (regression tests). Your solution should be capable of generating the very same results than the existing one. At this stage, don't you care about whether or not the results are right. The first goal is to refactor, not to fix bugs. If the existing solution says "2+2=42", your solution should too. If it does not throw exceptions, yours should not either. If it returns nulls, yours should return nulls too. And so on. Otherwise, you will be compromising 25k lines of code.

This is for the sake of the retro-compatibility.

Why? Because right now, It's your unique guarantee of a successful refactor.

And many of the unit tests either directly reference that interface, or are coupled to base classes which reference that interface.

A way to guarantee the retro-compatibility is urgently needed for you So here your first challenge. Isolate the component for unit testing.

Keep in mind that those 25k lines of code were made assuming the possible results of the existing code. If you don't break this part of the contract, then you are half way to the final solution. If you do, well: may the force be with you

Once you have designed and implemented the new "contract", replace the old one. Deprecate it or take it out.

I have suggested leaving bugs alone because of refactoring and fixing bugs are different tasks. If you try to take them forward together you may fail on both. You may think you found bugs, however, they could be "features". So leave them alone (for a minute).

25k lines of code seem to me enough issues to make me focus on only one task.

Once your first task is done. Expose those bugs/features to your boss.

Finally, as @Stephen has told:

There's not much you can do about it except slowly make things better. When you create the new classes, make sure you properly test them and create them using SOLID principles so they will be easier to change in the future

Improve this answer
5

Test it.

Everyone else is recommending how to refactor so there is little impact. But with that many errors, even if you succeed in refactoring with as little as 10 lines of code (you probably can), then you have impacted 25,000 code flows, even if you didn't need to rewrite them.

So, the next thing to do is make sure your regression test suite passes with flying colors. And if you don't have one, then make one that will. Adding a comprehensive regression test suite to your monolithic project sounds boring but is a nice way to increase confidence in release candidates, as well as release them faster if the suite is automated well.

Improve this answer

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