2012/6/14 Tommaso Teofili <[email protected]> > Hi Thomas, > > big +1 to your comments, I also encountered some of the things you > mentioned, that's why I opened . >
missed to paste the link to https://issues.apache.org/jira/browse/HAMA-572 > I think that we should now release 0.5.0 and then plan for an appropriate > API / code review. > Just for the sake of an example, the BSP interface (a very important one) > has generics which don't have neither binding to interfaces nor > documentation to explain what K, V, etc. mean. > > We may want to use the ASF Sonar instance to enable a semi automatic way > of checking for this kind of bugs. > My 2 cents, > Tommaso > > 2012/6/14 Thomas Jungblut <[email protected]> > >> Hi all, >> >> I'm currently going through all packages and cleaning up code by eclipse >> warnings. There I found some huge bugs, for example: >> >> public final void add(List<Metric<? extends Number>> metrics){ >> > metrics.addAll(metrics); >> > } >> >> >> Which is basically not intended, but wanted to reference a field name >> "metrics". This part is just adding the same collection to itself. >> Also I have seen many not using override annotations or using generics. >> There are also many open resources that are not properly closed with >> finally, thus possible descriptor leaks. >> There are several places, also I did some faults because several warnings >> are not turned on in the latest eclipse version. >> >> I would advise you (at least the comitters) to turn on the code >> inspections >> in your IDE. >> I have turned on several things in eclipse: (can be found in the project >> preferences or global preferences under Java/Compiler/"Errors/Warnings") >> >> From top to bottom: >> >> Non-static access to static member (WARNING) >> Indirect access to static member (WARNING) >> Method with a constructor name (WARNING) >> Parameter assignment (WARNING) >> Method can be static (WARNING) >> >> Serializable class without serialID (WARNING) >> Assignment has no effect (WARNING) >> finally does not complete normally (WARNING) >> Using a char array in string concat (WARNING) >> hidden catch block (WARNING) >> Inexact type match for varags argument (WARNING) >> Nullpointer access (WARNING) >> Compare identical values (WARNING) >> class overrides equals but not hashcode (WARNING) >> dead code (WARNING) >> >> Type parameter hides another type (WARNING) >> Method does not override package visible method (WARNING) >> interface method conflicts with protected object method (WARNING) >> >> Deprecated API (WARNING) // can be neglected by using annotations if not >> other possible though >> Forbidden references (ERROR) >> Discouraged references (WARNING) >> >> Value of local variable is not used (WARNING) // please delete it if never >> read >> Unused import (WARNING) // please format and organize imports before >> making >> a patch (in eclipse CTRL+SHIFT+F and CTRL+O) >> Unused private member (WARNING) // remove if never used >> Unnecessary cast or instanceof operation (WARNING) >> Unused break or continue label (WARNING) >> >> Unchecked generic type operation (WARNING) >> Usage of raw types (WARNING) // sometimes can not be avoided, can be >> neglected via annotation >> Generic type parameter declared with a final type bound (WARNING) >> >> Missing override annotation (WARNING) >> Annotation is used as super interface (WARNING) >> Unhandled token in SuppressWarnings (WARNING) >> >> I will put this up in our wiki and I want that everybody who commits a >> patch is checking against these guidelines, I don't want to add Findbugs >> because the false positive rate is too high. >> Would be great if everybody will care about that, because static code >> analysis is very great and if it's offered we should use it. >> >> -- >> Thomas Jungblut >> Berlin <[email protected]> >> > >
