Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Burying your dead code

eng.grandrounds.com

Post reply on HN
Print view View original

41–49 of 49 posts

Re: Burying your dead code

#41

Post by khedoros1 »

reificator wrote: post #30
khedoros1 wrote: post #16

Earlier quoted context omitted.

Reading through the discussion here: most of the comments are assuming interpreted languages. The article itself was talking about Ruby. And I'd imagine that in any form of client-server system, especially if there are multiple client implementations, it would be hard to tell if specific server endpoints are definitively "dead".

If you don't see any logged requests to that endpoint for a certain period of time, you can be pretty sure of its status. If you don't have that detail in your logs, that's step 0.

"Pretty sure" stinks, if you're coming from a world where the compiler can tell you "this code never gets called by anything".

Re: Burying your dead code

#42

Post by lomnakkus »

msangi wrote: post #40
tdriggs wrote: post #3

Static analysis is your friend here. TypeScript, Rust, and C# can all identify dead code and report errors or warnings from that, and can perform "find all usage" searches across a workspace.

Static analysis is very useful to identify unreachable code given that it can see all the code depending on it. It doesn't help with potentially reachable but unused cpdepaths, which in my experience make up for the most of 'dead' code. Think of the functionality in an application that nobody uses or of obsolete API version when all the clients use a more recent version.

This is an excellent point, but it should be noted that once you delete the unused API endpoint, finding the rest of the dead code is almost trivial with static analysis.

30-15 to static analysis, I think? ;)

Re: Burying your dead code

#43

Post by cassowary »

pascalxus wrote: post #6

PHPStorm/IntelliJ has this feature where it marks any unused functions as gray. If you want to find where used functions are used, simply hit Ctrl-G and it'll show you all the places.

Well, privates that are unused or only used by something like

    $method = 'get' . $command;
    $this->$method;
are marked in grey. Which means some methods that you need to keep are grey. Not necessarily helpful. And other methods you want to deleted are not grey.

(The solution to that is "write code that doesn't suck", but alas, dead code tends to be a bigger problem in code that you didn't write.)

But as others have said, the unused code that keeps me busy is the stuff that says:

    if($this->option->isTurnedOn()) {
        $this->doStuff();
    } else {
        $this->doThat();
    }
Sometimes it turns out that everyone uses it in the non-default state. Then there's the cases where everyone wants to use it in the non-default state, they just don't know the option can be toggled.

Re: Burying your dead code

#44

Post by cassowary »

abritinthebay wrote: post #15

People are saying "shouldn't tests/code coverage fix this" or throwing around "static analysis" like it solves this issue. Yes, static analysis is useful for finding internally orphaned code (though a simple search and seeing if the references outside of tests are 1 is just as effective) but that's not all dead code. The article is just using that as a simple example. If you have a feature that has non-orphaned metho…

I think the other problem is that static analysis is a great tool for a new code base. But a new code base has no dead code.

Or to put it another way, when you're designing a system, please think about the benefit static analysis will have in identifying dead code three years hence. But when you're trying to find dead code in that ten year old PHP code no amount of woulda's and coulda's will get rid of it.

The intelligent person solves the problem they've got. The unintelligent person shoulda's coulda's and woulda's you into oblivion.

(NB. I would probably choose a tool with a good type system, and I would try to write to its strengths, in any system I was designing today. But that still doesn't help me identify the dead code I've got.)

Re: Burying your dead code

#45

Post by cmcginty »

What is the point of finding the commit that removed the usage of your dead code? You should only care about the state of master. There is no point trying to prove where the removal happend.

Bisect is great to find the commit that caused a bug, because the commit narrows down the code to inspect to fix the bug.

Maybe I'm missing something here, but git bisect doesn't seem to apply in this case.

Re: Burying your dead code

#46

Post by reificator »

khedoros1 wrote: post #41
reificator wrote: post #30

Earlier quoted context omitted.

If you don't see any logged requests to that endpoint for a certain period of time, you can be pretty sure of its status. If you don't have that detail in your logs, that's step 0.

"Pretty sure" stinks, if you're coming from a world where the compiler can tell you "this code never gets called by anything".

When you're making a library, the compiler can't do anything of the sort.

Re: Burying your dead code

#47

Post by Doxin »

cmcginty wrote: post #45

What is the point of finding the commit that removed the usage of your dead code? You should only care about the state of master. There is no point trying to prove where the removal happend. Bisect is great to find the commit that caused a bug, because the commit narrows down the code to inspect to fix the bug. Maybe I'm missing something here, but git bisect doesn't seem to apply in this case.

Generally you'll want to understand why the code is dead before removing. Otherwise it might've been left for a good reason. Chesterton's fence[1] and all that.

[1]https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence

Re: Burying your dead code

#48

Post by ro_sharp »

slap_shot wrote: post #8

I don't know who to give credit for this concept, but I'm a fan of "tombstoning" functions that are being removed. You basically replace (or prepend) the body of the function with a call to a method that logs the timestamp, calling function, etc. Here's an example in PHP from a quick google: https://github.com/scheb/tombstone I once had to lead a large refactor of a project written in an foreign dynamically typed lan…

Another bloody good argument against using such languages for large projects.

Re: Burying your dead code

#49

Post by gknoy »

eduren wrote: post #36
chris_wot wrote: post #21

Earlier quoted context omitted.

If there was sufficient code coverage then the test would hopefully invoke a function that eventually calls on your potentially dead function.

I think the poster was saying that proper unit testing would give false positives. Example: If I have a "dead" add() function with a unit test that asserts add(2,2) == 4 , then code coverage would report that function being covered even if no other part of the codebase uses it anymore.

That's exactly what I was trying to say -- thanks!

I still _want_ that kind of test coverage, esp when there's weird edge cases, but when looking for dead code, it's probably good to look at only the acceptance / integration tests (or whatever you want to call the tests that do the holistic "I click here and everything works" tests).

Post reply on HN