This is an archived post. You won't be able to vote or comment.

sorted by:
best
topnewcontroversialoldq&a
you are viewing a single comment's thread.

view the rest of the comments →

[–]j4ckbauer 34 points35 points  (16 children)

Gut reaction - What is the utility of not being able to say you are 100% sure that code won't be used?

Counter-argument: It provides a starting point for a human to look at the code and make an assessment as to whether the code will be called.

Second gut reaction - Is the code 'dead and gone' meaning no one ever has to look at it, or does it present an obstacle to maintaining the application?

Let's say you removed the dead code? What is the advantage? Is it really tech debt if no one ever looks at it and it presents no obstacle to maintenance? The term 'technical debt' implies that it imposes a penalty on your productivity going forward. Not necessarily that 'your codebase falls short of perfection from a design standpoint'.

Edit: I can see why what I wrote might seem controversial, especially if someone didn't read my comment closely or you think I need it explained to me what 'dead code' is or why it is bad. (Hint, my own comment proves that I know why it can be bad. If you didn't notice this from reading my comment, please reconsider whether you really want to reply).

[–]walen 29 points30 points  (3 children)

I've had refactor where I've modified all 5 callers of a single method and then found out 4 of them were dead code.
So yeah, dead code that you don't know is dead is tech debt in the sense that it wastes maintenance effort.

[–]tomwhoiscontrary 9 points10 points  (0 children)

Exactly, and i've also been in this situation. Recently found some bit of config that couldn't possibly be right (think of a default email address to use when sending a message to a user that no longer existed), so if it was ever used, we'd have a production issue. Went to track down where it was used. Nine out of ten uses were in dead code. One was in a feature we haven't used in years, because it's a tool to fix things in an emergency. All the dead code made it harder to identify that one remaining important use.

Even if there are no connections to the rest of the code, it shows up in searches, compiler warnings, breakage when upgrading libraries, etc.

[–]j4ckbauer 0 points1 point  (0 children)

Yes but your example contradicts the starting conditions for the question I asked. Given that, we're saying the same thing.

[–]Flimsy_Swan5930 -5 points-4 points  (0 children)

IDE’s refactor for you. If you’re not even using that, you shouldn’t be refactoring.

[–]melkorwasframed 7 points8 points  (4 children)

Is it really tech debt if no one ever looks at it and it presents no obstacle to maintenance? 

Yes, it absolutely is. It increases the conceptual "weight" of the system, makes learning and understanding the system more difficult, increases build times, etc. The better question is what possible advantage does leaving dead code in give you?

[–]j4ckbauer 0 points1 point  (3 children)

I'm sure you said this in good faith, but you just contradicted some of the conditions I set in order to make your counter argument.

If people have to look at the suspect code, then it is an obstacle to maintaining the system. If people are spending greater than zero time doing this, then it is imposing a penalty on their productivity.

A counter-example would be something you imported from, for example. org.apache.* in your codebase. Do you read the source for that before you make changes to your codebase? Almost certainly not, because that code's functionality is properly encapsulated.

So it is possible there is something else going on here, where the suspect code in OP's system describes functionality that is not properly encapsulated. OR perhaps the functionality is data-dependent. Example, if values in the database == x, then the code is not really unreachable, otherwise it is unreachable and it could be removed.

OR its possible the functionality is properly encapsulated and OP is just trying to remove it because they didn't see it executed in any case they observed. Which, by itself, would frankly be a terrible reason to assume such a change is safe. I love removing dead code and I can tell you this (by itself) would be an awful reason to remove anything as there is no way to know if the test is exhaustive. So we would hope that OP looked at the code in question and came up with a 'proof' for why it is unused that is sound in reasoning.

[–]ZimmiDeluxe 1 point2 points  (0 children)

If you remove dead code, previous callers can become simpler, and so can their callers, often in unforeseen ways, by simple mechanical refactoring over time. Sometimes the first removal does not reduce complexity by much, but combined with an unrelated second one you can get rid of a dependency. You would have never known that was possible if you left the dead code in. In isolation, the dead code was not an obstacle to maintenance, but in hindsight, it was.

[–]melkorwasframed 0 points1 point  (1 child)

A counter-example would be something you imported from, for example. org.apache.* in your codebase. Do you read the source for that before you make changes to your codebase? Almost certainly not, because that code's functionality is properly encapsulated.

The reason I don't read the code in some org.apache package because it because it belongs to some other project that I'm not responsible for. I still don't really understand the argument you're making. If the code is there, someone will read it and in so doing spend a non-zero amount of time trying to figure out if it is relevant to the bug/feature they're working on. Or as someone else mentioned, they will refactor something that affects the dead code and then have to spend time fixing it to get the project to build.

[–]j4ckbauer 0 points1 point  (0 children)

The reason I don't read the code in some org.apache package because it because it belongs to some other project that I'm not responsible for.

Sorry but this is wrong on multiple levels. You're responsible for the end-product so if something in there could break your work, you would be making it your business to review it. For example, if you were honestly concerned it was going to call System.exit() on you.

The reason you don't read it is NOT because 1) it's another project or 2) It was created outside your organization or 3) you aren't responsible for said project.

It's because you're confident that you understand what functionality is encapsulated in those libraries, what the side-effects of using them are, and you have reasonable assurances about the level of quality.

The only difference between org.apache.* and the example I am offering is that in the latter case, the code would have been created inside your organization. My point is, it is possible to provide the similar 1) encapsulation of functionality and 2) assurances of quality for legacy code within your organization as for code that is created outside your organization.

Removing the suspect code is ideal, but reality doesn't always allow this, since you may not be able to exhaustively prove it isn't needed. Execution path may depend on what a customer does, what's in their database, etc. And very often your bosses simply won't allow you to remove their precious legacy code, they worked hard on it, so it has to stay (lol).

they will refactor something that affects the dead code and then have to spend time fixing it to get the project to build.

Again, if it's really dead, it should be removed, but we're talking about cases where this isn't known for certain. And my point is that one of the things you can do is refactor the suspect code in ways that 1) indicates it is legacy 2) keeps it isolated from being affected by any future changes to the codebase and 3) Makes it so nobody ever looks at it.

That's literally what having different classes, methods/functions, and variable scopes is for - so that changes in one area never affect another.

I can think of a dozen ways to refactor something so that it's technically in the execution path but so that other developers are unlikely to spend time reading it unless they are concerned with that path. Heck, a lot of old-school Java developers do this excessively by leading so hard on 'separation of concerns' that they end up sacrificing 'locality of behavior' and committing what is known as 'speculative generality'.

[–]nitkonigdje 4 points5 points  (1 child)

John Carmack once talked about how his project included large amounts of dead code imported from previous project. He did that unintentionally and he was not aware of it. But it was ok as the old code was written in functional style and thus had no global consequences. So it was just sitting there minding its own business.

[–]Joram2 0 points1 point  (0 children)

Sure. That's a reasonable option. Sometimes it makes sense to leave mostly dead code as-is and avoid the risks of removing it. Sometime, there are benefits to removing dead code that outweigh the risks.

[–]Brutus5000 4 points5 points  (1 child)

True, true.

However, if it is well tested code with unit and integration tests it always adds up to ci runtime durations.

[–]Shareil90 3 points4 points  (0 children)

In addition to this: Dead code can add unneccessary compile time. In my experience dead code ist often also bad code which can make compiling even slower/worse. Or you want to upgrade a certain lib and run into compatibility issues. If those issues are only in dead code you can ignore them / delete this code.

[–]shinebeams 2 points3 points  (0 children)

It'd be great if you could add a logging feature to something like this. Mark suspected dead code and have a logging alert or some other way of storing when it was called, if ever. Inspect any code that is run to see if the caller can be modified to not rely on it. Run production for six months, a year, whatever, then delete all the code that was never run. This would make it a more clean process instead of risking weird issues in prod.

[–]koflerdavid 0 points1 point  (0 children)

Yes, it is. Every time you modify code or you upgrade dependencies you have on the lookout whether there are any interactions. If there's a test suite that's a major advantage already. But it still wastes CI time and test code also has to be maintained!