DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT <http://issues.apache.org/bugzilla/show_bug.cgi?id=35956>. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=35956 ------- Additional Comments From [EMAIL PROTECTED] 2005-08-04 05:59 ------- Ok, I'm *finally* looking at the current rules file :) Interestingly, this set of rules is considerably more comprehensive than what I was looking at the first time, so most of what I was suggesting in the attached updated file is already done... the attachment can be ignored I think. There are a few other modules I think I can justify activating though... I don't know how many more issues would be found in the current code base if they were turned on, I haven't tried... I'd be willing to bet not many for most of these because many of these are mistakes that I doubt many reasonably accomplished programmers would make, so a lot of these are "better safe than sorry" kinds of things... * StrictDuplicateCode Duplicate code is just plain bad... not in terms of something not working, although that is certainly possible in some situations, but it's just a sign of carelessness. If nothing else, I'm sure no one wants the Struts code base to show any carelessness that could easily be avoided. * AbstractClassName This one might not be one that can be activated because I can conceive of it breaking the public interface if it turns up any classes not properly named already... then again, it may very well find no such problems, in which case turning it on is good going forward. * ImportOrder Imports in alphabetical order just makes it a little easier to find if something is used. This is obviously not going to break anything, and is easy to fix with virtually any IDE in existence today (or an already-mapped macro in UltraEdit for us anti-IDE folk), so it's low-hanging fruit, might as well grab it I figure :) * CovariantEquals The CheckStyle docs say it all on this: "Mistakenly defining a covariant equals() method without overriding method equals(java.lang.Object) can produce unexpected runtime behaviour." * StringLiteralEquality I'd be very willing to bet there are no such mistakes in Struts, doing s == "test" when you meant s.equals("test") is indeed a novice mistake. Still, better safe than sorry I figure. * IllegalCatch Again, as the docs say: "Catching java.lang.Exception, java.lang.Error or java.lang.RuntimeException is almost never acceptable." In this cases where it actually *IS* acceptable, simply ignoring the error is acceptable. * PackageDeclaration This is almost a silly check frankly, but again, no harm no foul. * ExplicitInitialization It's kind of a code smell to initialize class members to their default values. I've also heard it argued that it introduces slight inefficiencies, but I'm not sure I believe that personally. * UnnecessaryParentheses I personally find code with parenthesis that aren't actually needed to be more difficult, some feel the opposite. If they are truly not needed though, it's just more characters for my brain to parse I figure. Those are my thoughts. If there is a consensus to enable any or all of these, I'd be happy to provide a patch, although it hardly seems necessary :) -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee. --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
