Coding style
This discussion was originally a part of a longer discussion that has been moved to Tech:Zipped topic content
Another thing, I have a few comments on the code style... while mostly I agree on the code style you described on the contributions page, I do have a few objections. First, tabs are not everybody's friend... you will never get a tabbed java source code align correctly with any other tab size than the original tab size of the author... and at this point the advantage of the tabs are gone. Believe me, I have experience with this one. Where I work we are a few developers with different coding style preferences, and in the beginning we had big fights about code style... now we settled down on the following principles:
- never use tabs, only spaces;
- it really doesn't matter where you put the braces, put them so the code is more readable;
- generally, readability beats any style standard;
- don't ever change the style of a code written by somebody else, i.e. additions to other's code should follow the style of the existing code, and never reformat existing code;
- format new code however you want as long as it is readable;
These work remarkably well for us... and believe me, at the beginning we did have some wild re-re-reformatting wars ;-)
Cheers, Csaba 28-Nov-2006 06:40 PST
- With respect to coding standards, that's always an area of dispute for developers :-) I'm open to discussing changes to the Coding Style, but without very good reason I don't want that guideline changed. I've worked on more than a dozen projects since 1998 ranging in size from just me to as many as 100 people, and standardizing coding style has always been a good thing. The statistics I've heard are that 80% of coding is maintenance of old code, and since it's often someone other than the original author that is doing that maintenance it's best to make the code as easy for them to understand as possible, and that includes ensuring that the coding style is consistent. I don't plan on ignoring contributions that don't follow the coding style, but those contributions will likely be reformatted to keep the code consistent. As to specific comments about tabs or placement of braces, those arguments are hopefully covered in the coding style guideline. -- Ryan 28-Nov-2006 11:06 PST
Method exit
PMD send nice warnings. There is one rule I'd like to discuss though: OnlyOneReturn
A method should have only one exit point, and that should be the last statement in the method. This rule is defined by the following Java class: net.sourceforge.pmd.rules.design.OnlyOneReturnRule
According to this rule
private static File retrievePropertyFile(String filename) {
File file = null;
try {
file = Utilities.getClassLoaderFile(filename);
return file; //there should not be an exit point here
} catch (Exception e) {
// file might not exist
}
try {
file = new File(Utilities.getClassLoaderRoot(), filename);
return file; //oops yet another exit point
} catch (Exception e) {
logger.severe("Error while searching for resource " + filename, e);
}
return null;
}
should be
private static File retrievePropertyFile(String filename) {
File file = null;
try {
file = Utilities.getClassLoaderFile(filename);
return file;
} catch (Exception e) {
// file migh tnot exist
try {
file = new File(Utilities.getClassLoaderRoot(), filename);
} catch (Exception e1) {
logger.severe("Error while searching for resource " + filename, e1);
}
}
return file;
}
What do you think? I think this rule is rather controversial and would be in favor of totally disabling this rule.
- There are a lot of PMD rules that I think are somewhat questionable, although overall it's a hugely useful tool. In this case I agree that the "single return" rule is particularly questionable - following that would lead to code like the following:
// good version public String processArg(String arg) { String result = "something"; if (arg == null) { return result; } // do something with arg if (!testArg(arg)) { return result; } // do more stuff... return result; } // PMD "single return version public String processArg(String arg) { String result = "something"; if (arg != null) { // we could have returned if null // do something with arg if (testArg(arg)) { // we could have returned if failure // do more stuff... } } return result; } - To my eye that's pretty horrid. In general having a single return is cleaner, but that's not true in all cases.
- Another note, but it may not be worthwhile to put a lot of annotations in the code for PMD - once an annotation is in the code it will probably never get removed, even if it becomes invalid. Overall I think PMD is useful, but my opinion is that there's no point in trying to get rid of all PMD warnings since some of them will obviously be bogus. -- Ryan 24-Jul-2007 12:52 PDT
- Having just re-read this I wanted to clarify that I'm not against having some PMD annotations in the code, but my preference would be that we don't use hundreds of them solely to achieve the goal of an error-free PMD report. We want to use PMD to generate better and more readable code, but I don't think it's helpful to extensively modify good code solely because PMD produced a warning. -- Ryan 25-Jul-2007 09:07 PDT
- I prefer quick returns instead of many assignments, too. It's even quicker and easier to read especially if you have a method with a few if conditions and return a value instead of assign them to a reference and return at the end. In some cases however I'm often just lazy and instead of throwing an Exception I return a null value. Mike 24-Jul-2007 12:57 PDT
XHTML
To ensure valid XHTML during development is a very hard task. Maybe you should try to use a tool like this servlet filter in the development phase:
- For a coding style guideline I think it's important to emphasize that any HTML in JSP pages (or elsewhere) should be XHTML compliant. However, the suggested filter might be useful as a validation tool similar to JUnit or PMD. Thanks for the pointer! -- Ryan 01-Apr-2007 12:42 PDT
Tabs vs. Spaces
One of the oldest wars in programming circles is whether tabs or spaces should be used for indentation. The Linux Kernel coding standard says tabs, the Java coding standards say spaces. Until recently I've been in the "tabs" camp and as a result the JAMWiki standard is tabs, but the job I'm working on now requires spaces, and I've found that modern editors make that far less evil then it used to be... so, does anyone have a preference? There's one vote for spaces instead of tabs above, and I'm the middle. If enough people prefer spaces to tabs I'm amenable to changing the project standard, otherwise we'll just stick to tabs. Speak now or forever hold your peace. -- Ryan 14-Jul-2007 17:20 PDT