Honest Code
Has this ever happened to you? You're sitting at your computer, mired deep in code, trying to understand what is going on, and trying to figure how to approach your changes.. and all of a sudden it strikes you - you're looking at a piece of code and you're not even sure it's being used any place!I have to admit that I've literally lost days of work dodging and working around problems caused by code that, well, simply wasn't being used any place. I don't think I'm alone.
When we look at code in a code base, the default assumption is that it is being used someplace. If it isn't, it's a subtle lie. It's something that could trip us up.
I think that, fundamentally, there are two different modes of software development. In one mode, we write libraries and frameworks. When we write libraries and frameworks, we don't know how much of our code is being used by our end-users so we have to assume it's all used. In the other mode, we write applications. When we write applications the only code we have to write is the code that we are going to use.
Application code has to be kept honest and the way to do that is to ruthlessly delete things that aren't being used. It's less work in the long run, and it's nice to know, when you look at a piece of code, that it is there for a reason and that you aren't wasting your time.
!commentForm
When we inherit code, it would sure be cool to have a cruft detector that would show you the unused functions in the code. If testing were sufficient then coverage tools would help. In Python, I've renamed functions in a rude way ("maybe_dead_oldname") and run the test suite we had, sometimes after grepping (sometimes not). Sometimes we've commented them out during testing, but grepping is often the best choice we have, and not certain all the time.
Maybe a new refactoring tool?
Tim
http://tottinge.blogsome.com
Maybe a new refactoring tool?
Tim
http://tottinge.blogsome.com
Hi,
Tim: If you're working in .NET, you can run FxCop[?] and it will tell you that a variable or method is never used. But that only works if they are private or internal. If it's public, you'll have to fall back to the maybe_im_dead_method.
My two cents,
Michael
Tim: If you're working in .NET, you can run FxCop[?] and it will tell you that a variable or method is never used. But that only works if they are private or internal. If it's public, you'll have to fall back to the maybe_im_dead_method.
My two cents,
Michael
- Try resharper instead. I think it tells you about any unused variable or method. I think it can also detect dead code within a function. Now if only it could tell you about logically dead code (i.e. if statements with branches that are never taken.) == UB
As a corollary to this, I find commented code to be another kind of lie. The lie is: "I might need this one day." I can't remember the last time I uncommented code that somebody else commented out.
Yes! I wish I'd added that too. Commented out code is a real peeve of mine. When I work with someone for the first time, I often have to have that conversation: "So, it's not being used, right? .. Okay, so we can delete it, and if you ever need it (haha) it's still in version control, right? .. Hey, look at how much easier it is to understand everything!" -- Michael Feathers
- Rule 1: Don't check in modules that have commented code.
- Rule 2: If you check out a module that has commented code, delete that code.
Yes! I wish I'd added that too. Commented out code is a real peeve of mine. When I work with someone for the first time, I often have to have that conversation: "So, it's not being used, right? .. Okay, so we can delete it, and if you ever need it (haha) it's still in version control, right? .. Hey, look at how much easier it is to understand everything!" -- Michael Feathers
ReSharper[?] also has a "Find Usages" function that you can run on a method or variable. You get a nice results pane with all of the calls to your method listed. A double-click takes you to the code where the call is made. It's a very nice feature.
My favourite anecdote about this involved an unmaintainable system that was probably 90% pointless code. At one point one of my colleagues was having difficulty checking out with CVS. Different code was being checked out on his Windows desktop machine and the Unix server. After investigation we realised that he was looking at different parts of the same 5000 line method. Because he was browsing the source on Unix with vi in a terminal window, he could only see 40 lines at a time and it was hard to work out exactly where in the code he was looking (turning on line numbers helped). And when we found what he was really looking at on the Unix box, it turned out to be in the middle of several hundred lines of code that had been commented out!
That has to be one of the weirdest ones I've heard in a while. Scary. -- Michael Feathers
That has to be one of the weirdest ones I've heard in a while. Scary. -- Michael Feathers
Our code base is still evolving at a fairly good clip. Often when starting a story, we use ReSharper[?]'s Find Usages feature to get a feel for what our changes will likely impact. Doing this on a regular basis has the added benefit of forcing us to think about whether the objects that are using this code we're planning to change should be using it. We might ask: are the names conveying the reason why this code is being used? Are the dependencies sound? Are any tests calling this (public) method? Occasionally, there are no usages, we delete the unused code and happily announce our accomplishment to the rest of the team. Deleting code just feels so good that we usually can't keep it to ourselves! ;-)
How dare you! Don't you know that code is a company asset? We spent good money paying people to write that code and if you delete it, it's just like throwing money away!
Just kidding :-) but it's amazing how many people have that attitude. They never take into account all the money they lose in time spent tripping over dead code. -- Michael Feathers
How dare you! Don't you know that code is a company asset? We spent good money paying people to write that code and if you delete it, it's just like throwing money away!
Just kidding :-) but it's amazing how many people have that attitude. They never take into account all the money they lose in time spent tripping over dead code. -- Michael Feathers
This is one reason you should use a code coverage tool such as Cobertura or Clover. If a method or section fo code isn't being tested, and can't be reached through the public API, then it can and should be deleted. I've discovered entire packages that were unreachable and unnecessary by using code coverage tools.
Elliotte: I fully agree with you, but the point is, if I was careful enough to write a unit test for my code, Cobertura (or whatever) will still tell me, that the code is tested and thus *possibly* useful, so that doesn't really help. I noticed, though, when playing around with the eclipse profiler, that the profiler keeps track of which methods are called when the system is running. Maybe an option could be to examine the log files from the profiler?
Code coverage can be really useful, not only for unit testing. It is possible to instrument classes/jars and measure code coverage in functional tests, system tests and even in production. Actually, at a company I worked for they instrumented parts of the code base they suspected was not used and measured production coverage. Imagine safely deleting 10 year old code that nobody understands anymore. O joy!
Of course if your are really extreme, you could send your code to Guantanamo: Do you have problems maintaining high test coverage? All code is guilty until tested innocent. Send the untested code to Guantanamo! http://docs.codehaus.org/display/ASH/Guantanamo
Emma is another nice open source code coverage tool: http://emma.sourceforge.net/
Of course if your are really extreme, you could send your code to Guantanamo: Do you have problems maintaining high test coverage? All code is guilty until tested innocent. Send the untested code to Guantanamo! http://docs.codehaus.org/display/ASH/Guantanamo
Emma is another nice open source code coverage tool: http://emma.sourceforge.net/
Can anyone recommend a tool which will detect commented Java code? I have a large code base that would be too big to check by eye.
I've been experimenting with PMD (http://pmd.sourceforge.net/) but there doesn't seem to be a rule for commented code. I'm considering writing one, but before I do, just checking no-one has done this already...
I've been experimenting with PMD (http://pmd.sourceforge.net/) but there doesn't seem to be a rule for commented code. I'm considering writing one, but before I do, just checking no-one has done this already...
>>steve hartley,
>>Can anyone recommend a tool which will detect commented Java code? I have a large code base that would be too big to check by eye.
Checkstyle will report JavaDoc[?] for you: http://checkstyle.sourceforge.net/config_javadoc.html
JavaNCSS also provides some JavaDoc[?] metrics: http://www.kclee.de/clemens/java/javancss/
>>Can anyone recommend a tool which will detect commented Java code? I have a large code base that would be too big to check by eye.
Checkstyle will report JavaDoc[?] for you: http://checkstyle.sourceforge.net/config_javadoc.html
JavaNCSS also provides some JavaDoc[?] metrics: http://www.kclee.de/clemens/java/javancss/
Hi Trond
Thanks for your suggestion, but what I was really looking for was a tool that looks through comments, and detects if each comment contains a genuine comment ie English text (a good thing!) or whether instead it is code that has been commented out of the compiler's view, when in fact it should be deleted (a bad thing! [IMHO])
Steve
Thanks for your suggestion, but what I was really looking for was a tool that looks through comments, and detects if each comment contains a genuine comment ie English text (a good thing!) or whether instead it is code that has been commented out of the compiler's view, when in fact it should be deleted (a bad thing! [IMHO])
Steve
Add Child Page to HonestCode