Replacing Humans with Software Inspectors 90
An anonymous reader writes "What if you were able to perform a portion of your code reviews automatically? In this first article of the new series 'Automation for the People', development automation expert Paul Duvall begins with a look at automated inspectors like CheckStyle, JavaNCSS, and CPD. The piece examines how these tools enhance the development process and when you should use them." From the article: "Every time a team member commits modifications to a version control repository, the code has changed. But how did it change? Was the modified code the victim of a copy-and-paste job? Did the complexity increase? The only way to know is to run a software inspector at every check-in. Moreover, receiving feedback on each of the risks discussed thus far on a continuous basis is one sure-fire way to keep a code base's health in check automatically!"
CPD == copy/pasted code detector (Score:5, Informative)
There are some examples of CPD output there too - like the duplicate code chunks that it found in the JDK source code and in the Apache httpd source code.
For much more on CPD, see chapter 5 [pmdapplied.com]!
CPD For Slashdot Posts (Score:4, Funny)
I bet the numbers would be off the charts!
Re:CPD For Slashdot Posts (Score:1, Funny)
Re:CPD == copy/pasted code detector (Score:2)
Re:CPD == copy/pasted code detector (Score:2)
"way to keep a code base's health in check" (Score:1, Insightful)
Uhhhh, maybe you should try an english language cliche parser instead?
Just don't get lazy (Score:5, Insightful)
As a tool to point out obvious code flaws, catch conflicting code in large projects, etc. this is great. But I've found over the years that if you don't understand how to do something manually and aren't able to second guess a tool when it makes a mistake then these types of things can end up being more trouble then they're worth.
Just my $.02
Re:Just don't get lazy (Score:2, Insightful)
Re:Just don't get lazy (Score:4, Insightful)
My point isn't that you made a typo and that you should feel bad, it's that everyone makes mistakes, including people who know what they're doing. That is why tools like this are useful.
Re:Just don't get lazy (Score:4, Interesting)
Re:Just don't get lazy (Score:1)
Re:Just don't get lazy (Score:2, Offtopic)
Re:Just don't get lazy (Score:2)
But I really never felt like I understood what I was doing unitl I started writ
Re:Just don't get lazy (Score:1)
Re:Just don't get lazy (Score:2)
Re:Just don't get lazy (Score:2)
I'm not saying tools, IDE and even M$ style code generators are a bad thing. But it is important to understand them under the hood. They are each a great asset when they work correctly or completely fill your needs. When they fail or can't do that last 5% because the tool maker didn't anticipate your particluar need then your only recourse is to open the hood and fiddle with the bits. I cou
Re:Just don't get lazy (Score:2)
Re:Just don't get lazy (Score:2)
Re:Just don't get lazy (Score:2, Funny)
Re:Just don't get lazy (Score:2)
Automation and the job market (Score:1)
Automation can only go so far. This is a rubber band phenomenom that will snap back into proper size when automation turns out to fail.
Failure is a guaranteed and major part of any effort to convert analog to digital. Programming is highly analog, believe it or not - i
Re:Automation and the job market (Score:2)
The people who end up being the ones to fix all the bugs that got by the automated software checkers?
Re:Automation and the job market (Score:2)
Umm, that's really rather off what most people observe in reality. While in the limit the directly-written machine code (MC for short) might be better than anything out of a compiler, the level of guru-dom required to do this on modern architectures is really ridiculou
Re:Automation and the job market (Score:2)
The article's author, Paul Duvall... (Score:2)
How about FindBugs? (Score:5, Informative)
Re: How about FindBugs? (Score:2)
Not that source-level analysis is useless, of course; there's a lot of low-level style that it can check. But IME, it can be more trouble than it's worth, because it doesn't understand that there are times when it
The only way (Score:5, Insightful)
Or you can hire people who are good and who you trust. A real human will do a better job of 'code reviewing' every time, and if you hire good people, then you don't need to worry about what they commit. An occasional check should be enough to make sure you haven't accidentally hired a loser. (Also human code reviews, if done correctly, are great because they help everyone become better programmers).
This is easy to do in small companies, but somewhat harder in big companies. Still, if I were CEO, and my managers started using this tool, I would get worried and start thinking seriously about how to change the company culture. You don't want your company to become like Qualcomm, or Novell, where bureaucracy rules the day.
Re:The only way (Score:3, Insightful)
> I would get worried and start thinking seriously about
> how to change the company culture.
I'm not sure that using static analysis tools are a sign of a bad corporate culture. I think they're just one more safety net you can use to find code problems. This is especially true for something like CPD, which finds duplicated code anywhere in the codebase - checking for this sort of thing manually is pretty difficult.
That said, I agree t
Re:The only way (Score:5, Informative)
Now, lint tools aren't always right, so there were many places where we had to add comments in the code to get the tool to ignore the next lines. The important thing about the tool is it is "double checking" that you meant to do it that way. If you do, you add the ignore comment, and get to discuss it in an inspection. In this way it enhanced the inspection process instead of detracting from it.
Fortunately, I worked in an area where the quality of the code was considered important by the developers and management and if code wasn't ready, it didn't make it into the build. Simple as that. Of course, if you were going to miss the targeted build, management wanted to know why, but most of them would listen to you. (They might also ask you to work on the weekend to get it done...)
Now, replacing the mundane, manual inspections with tools is just stupid. Yes, in some places it can be done, but for the most part it is a horrible idea. Humans are better than software at inspecting code. Tools may be faster, but humans are better. Humans can catch mistakes like passing an incorrect variable or returning the wrong value. They can also examine any requirements or design documentation and determine if you are doing the correct thing. If nothing else, at least they are familiar with the overall application (or should be). Maybe you are making a window too big. It looks fine on your machine but there is a requirement that it works at 1024x768. You don't notice since you have a big monitor. The tool can't notice it since it doesn't read requirements documents. The requirement may not exist outside your memory of a short chat with the customer.
The reality is that software inspections SAVE time. No one believes it. Or, if they do, they forget about it because of "crunch time". Sorry. Unless you are coding trivial or simple applications it doesn't pay off. You can argue all you want that you can get around it, or there are better ways, but I don't believe you. I have seen the data from Michael Fagan's study at IBM, and inspections work. Motorola actually published the results of their switch to the Fagan Methodology and found that development time was reduced, fewer bugs were introduced and that more features could be added. After that, they made it company-wide policy to use the Fagan Method.
So why does this method work? Well, Fagan conducted over 11,000 inspections when he was at IBM to develop this methodology. It took a few years to conduct them all and analyze the results, but he found a great way to reduce bugs, cost and development time. So, unless you have the formal data to backup your claims, (anecdotal evidence doesn't count), I'll keep claiming that inspections are better. Proper inspections take preparation, focus and effor, but they make you better off.
Re:The only way (Score:2)
It's simple division of labor: c
Re:The only way (Score:2)
This is not an inspection. It is a desk check. A proper inspection, by Fagan standards, requires 1 hour of preparation for 100 lines of code. If they aren't noticing or looking at the "simple" stuff, then they are going to miss "real" mistakes. Also, you don't hold an inspection after you finish coding your stuff, you hold it after you have gone over the checklist created for "inspection ready". This may include th
Re:The only way (Score:2)
If the requirement isn't captured somewhere, it's not a requirement... it's a suggestion. Other than that, I agree completely with the rest of your post.
Re:The only way (Score:1)
Considering TFA states "The only way to know is to run a software inspector at every check-in", this is a sign of using it as a crutch. That and the fact that the GP states that they had to add comments to get their tool to ignore "valid" pieces of code, that's an even bigger issue.
And just exactly what are you checking with the "code-inspector"? Are you being a "grammar nazi" and checking for the ever important whitespac
Re:The only way (Score:5, Insightful)
Or you could do both!
Automated QA tools are cheap insurance against mistakes, and I'm surprised by the resistance to them I see in these comments. No, of course no one likes out-of-control bureaucracy, but that's not an argument against using automated tools to check your code.
Re:The only way (Score:2)
So of course they spent thousands to buy a test package any
Re:The only way (Score:2)
If I was CEO and my developers were not using these tools, I'd be looking to hire some high quality developers.
Fact: Code reviews improve code quality.
Fact: 80% of the cost of bespoke software occurs during maintenance.
Fact: No software developer is perfect.
Maybe you're arrogant enough to think you don't need these tools. I say that you do need them, your teams need them, and that you should be doing proper code reviews as well.
One of my software engineers has just updated the automated code build (which ru
Re:The only way (Score:2)
Re:The only way (Score:2)
hiring good people is not enough (Score:1)
and if you hire good people, then you don't need to worry about what they commit. An occasional check should be enough to make sure you haven't accidentally hired a loser.
This is still a commonly held belief, but it's just not true.
Allow me to quote from a 2004 OOPSLA paper: "we have found that even well tested code written by experts contains a surprising number of obvious bugs." (Link to entire paper is here [sourceforge.net].)
Even very good programmers make mistakes sometimes, and some of them are simple enough to
Re:The only way (Score:2)
Which is why you're not a CEO.
Real humans make mistakes. The best developers may be less likely to make big mistakes and introduce logic bugs, but even they will commit simple errors from time to time. So there is always a need for review, be it human or automaton.
It is not easy to find "the best" developers, or to distinguish them from above average developers (and average isn't very good). It was recently shown that "the best" people in their field consistently rate themselves lower than the set of
Re:doesn't work (Score:2)
I for one... (Score:2, Funny)
Brings new meaning to the phrase... (Score:3, Funny)
Code Revue (Score:2)
You mean like "gcc project.c"? Without that, I'd have to be in marketing
Re:Code Revue (Score:2)
In soviet Russia... (Score:2, Funny)
Bad Robots Work Too Hard (Score:2)
I want the code to be copy/paste every time, if it works and is maintainable, rather than sparkling new code. Why would I want some robot enforcing some need to reinvent the wheel every time I need to roll?
If this thing is going to be smart, it would look at code to replace with code elsewhere in the repository. I'm tired of doing that myself, and not copy/pasting enough. By which I mean factoring common code into its own scope, then pasting a refer
Re:Bad Robots Work Too Hard (Score:2)
However, there are plenty of times that copying and pasting chunks of code makes good sense.
Re:Bad Robots Work Too Hard (Score:2)
Re:Bad Robots Work Too Hard (Score:1, Flamebait)
"By which I mean factoring common code into its own scope, then pasting a reference to it."
Sometimes known as "object oriented programming". Since I helped Apple Computer move from their Pascal toolbox to C++ over a decade ago, I'd say that I know more about OOP than you'll ever know about anything.
Re:Bad Robots Work Too Hard (Score:2)
Oh yeah! I've been using OO for 15 years, therefore I know more about it then you know about anything.
Jeez dude, check the ego. No one cares how long you've been programming, and it doesn't prove jack about someones skills. It certianly doesn't mean that you know more about it then the other person knows about anything.
Re:Bad Robots Work Too Hard (Score:2)
Don't give me any crap about my ego when I'm slapping back some bitchass AC. It says more about your ego than anything else.
FWIW, you think I started out in OOP
Re:Bad Robots Work Too Hard (Score:2)
Re:Bad Robots Work Too Hard (Score:2)
CStrawman (Score:2)
I just posted about how copy/paste isn't always bad. I mentioned an OOP reason why, which is also just a good programming reason. You then went nuts.
Maybe if you start having these debates with yourself out loud, or wi
Re:CStrawman (Score:2)
Re:Bad Robots Work Too Hard (Score:2)
Instead of writing long lines of if-then-else we try to write something shorter that can do the same thing, faster and with hopefully less time and effort spent (writing and maintaining). A squeezing from different directions (with different priorities).
I think copy and paste coding is fine, if you suspect that the "duplicates" are likely to need to become different later, and especially if you're not sure in what way they are going to be different, just
Re:Bad Robots Work Too Hard (Score:2)
Missing alot (Score:2)
Instead of depending on style checkers for formatting, just get a good programming editor
and configure it for the style. This may not take care of all of it, but it can help.
I'm a big believer in lint and PCLint , which also can be used for style checking. I don't know how good JLint [artho.com].
The piece skipped out on automated testing -- ie. Purify.
They missed BoundsChecker.
Re:Missing alot (Score:2)
It's pretty good, although I had problems getting it to compile with recent Fedora releases. I think I ended up compiling it in a RH9 VMware partition or something. But once it's working, it's cool - it does some good dataflow analysis stuff and is nice and fast.
Oh, good, the silver bullet at last (Score:3, Insightful)
But this. This is different. Totally different. It's the real thing this time.
Re:Oh, good, the silver bullet at last (Score:2)
I don't think anybody's claiming this is a silver bullet.
What this does represent is an opportunity for incremental improvement.
Or just keep doing the same stupid thing that doesn't work very well. Your choice.
Re:Oh, good, the silver bullet at last (Score:1, Funny)
dpbsmith: Oh, good, the silver bullet at last. We've experienced those brief and tempestuous infatuations with flowcharts, Warnier-Ott diagrams, top-down programming, structured programming, Jackson structured programming, source code control, the waterfall model, Royce's Final Model, the spiral model, the sashimi model, object-oriented programming, CASE tools, Rational Unified Process, SEI'
Be smart (Score:4, Insightful)
Don't trust the code inspector to enforce a policy (except maybe coding style).
There's a lot of boilerplate code that goes into a program, and a code duplication monitor will cause all sorts of headaches in this area.
The same problem exists with comment checkers. If code is written properly, there is usually very little need to comment inside most methods. The name and javadoc will give more than enough description of what the method will do (you DO use javadoc, don't you?)
It's only as the method's complexity increases to a point where it's not blatantly obvious what's going on inside that you need comments at that level.
I've had too many managers force me to comment like this:
for(Iterator iter = files.iterator(); iter.hasNext();
{
File file = (File)iter.next();
if(file.lastModified() > new Date().getTime())
{
throw new ModifiedInFutureException(file.toString() " + has a modification date in the future");
}
}
Ok it looks much uglier after running through Slashdot's dumb filter, but you get the idea.
The above code in reality needs no comments whatsoever, except perhaps a single line at the top saying "Disallow modification dates in the future", but a bad policy caused ALL code checked in to comply with silly regulations, resulting in countless hours lost.
In truth, the date check code itself should have been implemented as a policy class to be added to the verify method, but I digress.
Re: Be smart (Score:2)
In that situation, I'd ask why all those comments are needed. If the answer isn't "To make the code easier to understand", then I'd refuse. And if that is the answer, then I'd point out that no half-way competent developer would have the remotest trouble getting all that straight from the code, so none of that is achieving the state aim. If
Re: (Score:1)
Re:So... (Score:2)
A similar method for HTML & CSS (Score:1)
Article is right on (Score:3, Informative)
This makes our reviews more productive, because people don't get caught up in discussions over curly brances, finding copied code, issues with contructor and exception idioms, etc.
The slashdot crowd is going to envison pointy-haired bosses basing performance reviews on this kind of stuff, which is a legitimae fear and shouldn't happen. Used in the hands of real software engineers though, this is similar in spirit to the woodworker's adage "measure twice, cut once". Loosely applied, "You know what you are doing, but be your own safety net".
-db
Check globally, not locally (Score:2)
The trouble with most of these tools is that they're aimed at local coding style issues, not global problems.
Typical global problems that are potentially machine-checkable before execution are:
Re:Check globally, not locally (Score:2)
How it should be (Score:1)
-=Robo=-
Re:How it should be (Score:2)
Now add tabs, carriage returns, etc it becomes painfull.
Plus, having a standard way to format code makes maintenance difficult, and can foobar automated documentation.
whitespace (Score:1)
Quibbling over formatting is silly. White space formatting doesn't affect functionality of the code at all.
fyi, FindBugs [sourceforge.net] doesn't look at the source code at all, only at the compiled bytecode.
Re:How it should be (Score:2)
It might be harder than you
The only software inspector you need! (Score:2, Funny)
Virtual Richard M. Stallman
The vrms program will analyze the set of currently-installed packages on a Debian GNU/Linux system, and report all of the packages from the non-free tree which are currently installed.
Future versions of vrms will include an option to also display text from the public writings of RMS and others that explain why use of each of the installed non-free packages might cause moral issues for some in the Free Software community. This functionali