On Tue, Aug 10, 2010 at 9:09 PM, Jeremias Maerki <d...@jeremias-maerki.ch>wrote:
> fine-tuning CheckStyle is probably a good thing. oh, i completely agree on that, and i'm sure we will find that opportunity as time permits, and when it does, i will happily contribute a number of comments for consideration; but that conversation is precisely what i wish to avoid at this juncture, since it will undoubtedly prove to be quite subjective; for example, how long should a line of code be permitted to be? the currently written FOP checkstyle rule is 100 characters, but why not 80 or 132? my own coding style is to use a single line for an expression regardless its length: the fact that my editor (emacs) happens to wrap at 200 characters is merely a property of my favorite font, screen resolution, window size, and wrap configuration; did I follow the rule in my new complex script code? no, i used my favorite style and disabled this warning; but i didn't make the decision randomly, and i didn't want to leave a visible warning during style checking; the CS keywords remain in the code, and can be used to evaluate or audit how far real practice diverges from an artificially prescribed practice (the encoded rules); what is my reasoning? artificial line breaks make code harder to read and understand, or at least that is my considered conclusion having read and written much code (though I would make an exception for APL - short lines are definitely better there); on the other hand, while fixing the current code, there were occasions where reported style violations marked possible coding errors; for example, the lack of a default clause in a switch statement was something i encountered a few dozen times in this cleanup work; in some of the cases, it was not a semantic error, but in others it was a semantic error waiting for a bug report; although I did not address findbugs errors in this pass, there are a few errors that it detects that can easily lead to semantic errors, such as copying or returning references to live arrays (as opposed to performing deep copies); in some cases, it is desirable, for performance reasons, to return a reference to a live array typed member of an object; however, in other cases, it is an invitation to unintentional side effects leading to dynamic errors; in these cases, a contributor or committer has to make a decision based on a deeper knowledge of code function and usage, and not merely upon the surface form; i saved the task of fixing findbugs errors (and PMD errors) until the low hanging fruit has been picked, namely the subject of the current patch; having plucked the easy ones, i'll continue to the more tricky ones, but that will have to wait until the current patch is in the trunk and no regressions appear; g.
