On Fri, Sep 10, 2010 at 5:52 AM, Sean Owen <[email protected]> wrote: > As you might see, the number of such warnings has come down from thousands > to hundreds.
This is really great. It's pretty cool to see the big drop on the hudson page: https://hudson.apache.org/hudson/view/Mahout/job/Mahout-Quality/ Does anyone have an idea why you can't view the warning details in hudson? The following page gives counts but the detail tag panes always are empty for me: https://hudson.apache.org/hudson/view/Mahout/job/Mahout-Quality/267/pmdResult/ > 1. One common one is "overrideable method called in constructor". This is a > reasonable point. > B breaks A by subclassing. Making the method or class final solves it, which > is my preferred solution where I do not see any reasonable case for > subclassing. Or, the constructor can be rearranged to call this logic > directly instead of via an overrideable method. Sounds like private or final would make sense here, unless there's no other method that needs to call the initialization method, in which case it is likely the logic should be placed in the constructor directly. > 2. Another is complaining about methods that throw 7 or more exceptions. > There are a number like this and it strikes me that it can't really be the > best thing. They could be wrapped in a generic exception, or, in cases where > the caller can't reasonably do anything, try to log and handle the condition > internally. It seems like it would be reasonable to create one or more wrapper exceptions as the need may arise. Perhaps MahoutException is a start. Algorithm specific exceptions like ClusteringException or ClassifierException could inherit from MahoutException and would read better, but I wonder whether they are overkill. > 3. Last one I see a lot is the "really long method" warning. I agree with it > but not sure how much it's worth chopping up methods. Good to think of when > refactoring though. I think keeping these warnings around until such methods can be refactored makes sense. Drew
