Veranderende wensen en eisen van de opdrachtgever zorgen er voor dat programmacode continu wordt aangepast en bijgeschaafd. Het gevolg van deze aanpassingen is dat de structuur, kwaliteit en leesbaarheid van de code er niet op vooruit gaat. Frederick P. Brooks vat dit principe als volgt samen:
All repairs tend to destroy the structure, to increase the entropy and disorder of the system. Less and less effort is spent on fixing the original design flaws; more and more is spent on fixing flaws introduced by earlier fixes. As time passes, the system becomes less and less well-ordered. Sooner or later the fixing ceases to gain any ground. Each forward step is matched by a backward one. Although in principle usable forever, the system has worn out as a base for progress.
Om dit verschijnsel enigszins tegen te gaan dient de kwaliteit van de code en architectuur op regelmatige basis gecontroleerd te worden. Code reviews door teamleden kunnen hiervoor gebruikt worden, maar nemen vaak enige tijd in beslag. Daarnaast is het een grijs gebied welke code-constructies nu wel en niet het melden waar zijn; wat de ene ontwikkelaar een prima oplossing vindt, kan een ander weer inefficient of lastig te lezen/begrijpen vinden.
Om dit verschijnsel enigszins tegen te gaan dient de kwaliteit van de code en architectuur op regelmatige basis gecontroleerd te worden. Code reviews door teamleden kunnen hiervoor gebruikt worden, maar nemen vaak enige tijd in beslag. Daarnaast is het een grijs gebied welke code-constructies nu wel en niet het melden waar zijn; wat de ene ontwikkelaar een prima oplossing vindt, kan een ander weer inefficient of lastig te lezen/begrijpen vinden.
Een zeer nuttige tool om tijdens de implementatie- en onderhoudsfase van een project toe te passen is FindBugs. FindBugs voert een statische code analyse uit: FindBugs herkent meer dan 350 patronen in de Java bytecode (en analyseert dus niet de “leesbare” Java code). Afhankelijk van de groote van de code base duurt deze analyse een paar seconden tot enige minuten.
Uiteraard is er een FindBugs Eclipse plugin beschikbaar, zodat FindBugs direct tijdens ontwikkeling of refactoring de code kan analyseren. De “high priority” bugs die gedetecteerd worden zijn vrijwel altijd de moeite van het reviewen waard. Minder belangrijke “bugs” hebben meer te maken met stijl, overbodige code of bad practices. Het hangt dan af van de context of deze detecties ook echt bugs zijn die opgelost moeten worden.
Naast integratie met Eclipse kan FindBugs worden gebruikt tijdens een Maven build. Voeg de FindBugs Maven Plugin toe aan de pom.xml:
<project>
...
<reporting>
<plugins>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>findbugs-maven-plugin</artifactId>
<version>2.3</version>
</plugin>
</plugins>
</reporting>
...
</project>
Run mvn site om de Maven-FindBugs rapportage te genereren.
Veel (open source) projecten hebben de FindBugs rapportage in hun daily build opgenomen. Veel van die build resultaten staan online en daardoor is het mogelijk de bijbehorende FindBugs rapportages in te zien. Een van de ernstigste bugs die FindBugs kan detecteren is het vergelijken van twee verschillende typen Objecten met equals():
EC: Call to equals() comparing different types
EC_UNRELATED_TYPES: This method calls equals(Object) on two references of different class types with no common subclasses. Therefore, the objects being compared are unlikely to be members of the same class at runtime (unless some application classes were not analyzed, or dynamic class loading can occur at runtime). According to the contract of equals(), objects of different classes should always compare as unequal; therefore, according to the contract defined by java.lang.Object.equals(Object), the result of this comparison will always be false at runtime.
De equals() methode die instanties van verschillende classes vergelijkt geeft altijd false terug. Voorbeelden van deze bug zijn met een simpele Google search makkelijk te vinden in online FindBugs rapportages:
it.imolinfo.jbi4corba.jbi.wsdl.Jbi4CorbaExtPreprocessDeserializer (regel 52) vergelijkt een java.lang.String met een javax.xml.namespace.QName. Regels 53-86 worden dus nooit uitgevoerd en de unmarshall() methode zal altijd null teruggeven, met alle (onvoorziene) gevolgen vandien.
org.itracker.web.actions.admin.project.EditProjectScriptFormAction (regel 119) vergelijkt een java.lang.Integer met een org.itracker.model.WorkflowScript. Hier had waarschijnlijk getId() achter moeten staan, want nu worden regels 120-123 nooit uitgevoerd.
Oneindige loops
IL_INFINITE_RECURSIVE_LOOPs worden ook gedetecteerd door FindBugs. In verschillende projecten kunnen mooie voorbeelden gevonden worden, vaak veroorzaakt door onzorgvuldig copy-paste werk. De org.apache.shale.remoting.logger.DefaultLogger kan beter niet gebruikt worden, omdat op maarliefst zes plaatsen in deze class een “infinite recursive loop” zit; zie regels 137, 151, 165, 179, 193 en 207. De oorzaak en oplossing zijn waarschijnlijk wel duidelijk (voeg "logger."toe voor elke van de genoemde regels), maar deze bug had voorkomen kunnen worden. Het Apache Shale project is trouwens geen actief project meer.
Andere recursive loops kunnen gevonden worden in
com.atlassian.seraph.util.CookieUtils (regel 117) en
org.jasig.portal.channel.dao.jpa.ChannelDefinitionImpl (regel 257).
“Gelukkig” eindigt een infinite recursive loop nog in een java.lang.StackOverflowError. Erger is het als een oneindige (while-, do-while- of for-) loop uitgevoerd wordt, zoals in
org.eoti.ai.genetic.gp.RandomNumber (regel 34)
org.jquantlib.math.statistics.ConvergenceStatistics (regel 99) en
ycook_lab.javatools.utils.ByteBuffer (regel 418)
met 0 als waarde voor de initialCapacityparameter).
Onmogelijke casts
BC_IMPOSSIBLE_CASTs worden meestal al gedetecteerd door de compiler. Er zijn desondanks constructies die altijd een ClassCastException opleveren, maar die niet door de compiler gesignaleerd worden, zoals:
public String[] getArray() {
Map map = new HashMap();
return (String[]) map.keySet().toArray();
}
public void doValue() {
Object obj = 1;
String s = (String) obj;
}
FindBugs zal een “impossible cast” detecteren in regels 3 en 8. Een goed voorbeeld van deze bug bevindt zich in de
org.microemu.applet.CookieRecordStoreManager (regel 151) en
org.jasig.portal.lang.ChainedError (regel 321 en 521).
Onvermijdelijke NullPointerExceptions
NP_ALWAYS_NULL komt regelmatig voor, maar wordt pas bemerkt tijdens runtime, of op z’n best tijdens unit tests. Voorbeelden zijn te vinden in
com.intel.bluetooth.BluetoothStackBlueZ
(regel 760, NPE indien serviceHandles == null)
com.intel.bluetooth.obex.OBEXSessionBase
(regel 243, NPE indien authChallengesSent == null)
fr.paris.lutece.util.filesystem.FileListFilter
(regel 75, NPE indien _strExtension == null)
com.gridsystems.config.tools.swing.PasswordField
(regel 99, regel 98 had moeten zijn if (resourceBundle != null))
fr.paris.lutece.plugins.digglike.business.TagSubmitDAO
(regel 129, de tagSubmit variabele in regel 120 is null)
Ongebruikte return waarden
RV_RETURN_VALUE_IGNORED van methoden van immutable classes zoals java.lang.String en java.math.BigDecimal zijn vaak de oorzaak van een ander type bug. Alhoewel de Java code er op het eerste gezicht goed uit ziet, moet niet vergeten worden het resultaat van de operatie toe te kennen aan de originele variabele:
// fout:
String s = "hello world ";
s.trim();
s.replace("world", "everyone");
BigDecimal b = new BigDecimal("100");
b.negate();
b.add(new BigDecimal("1"));
// goed:
String s = "hello world ";
s = s.trim();
s = s.replace("world", "everyone");
BigDecimal b = new BigDecimal("100");
b = b.negate();
b = b.add(new BigDecimal("1"));
Bugs zoals deze komen onder andere voor in de volgende classes: org.cyclopsgroup.jmxterm.cc.ConsoleCompletor (regel 63)
jp.sourceforge.talisman.xmlcli.builder.SaxOptionsBuilder (regel 197) en
org.teamweaver.is.backend.search.QueryManager (regel 1663).
Ongebruikte excepties
RV_EXCEPTION_NOT_THROWN is een soortgelijke bug, vaak veroorzaakt door te gehaast en onnauwkeurig werken of refactoren. Excepties worden wel aangemaakt (new ...), maar nooit gegooid (throw ...). Bijvoorbeeld com.funambol.server.notification.PushManager (regel 279 en 310) en
com.funambol.tools.test.ant.TestSyncMLBaseTask (regel 94).
Merk tenslotte op dat bovengenoemde classes en projecten al Maven 2 gebruiken, een automatische build hebben en FindBugs rapportages hebben en desondanks dit soort bugs bevatten. Projecten die geen (automatische) code reviews krijgen bevatten zonder twijfel een veel hoger percentage bugs, issues en problemen.
Naast statische analyse is meer nodig voor een volledige code review. Denk daarbij aan stijl en formatting, code conventies, code duplicatie en complexiteit. Kijk hiervoor eens naar CheckStyle, PMD en CPD, en JavaNCSS.
