This document examplifies how to conduct a source code audit with Lint4j.
The first step is to construct the source path. Change to the base directory of the project, and then look for the first package of the source code, in this case "com" (aka com.sun.j2ee).
/Users/Shared/smart_ticket2.0]% find . -type d -name src
./src
./src/app/client/midp/src
./src/app/server/ejb/src
./src/app/server/web/src
./src/app/shared/src
Then we need to find all archives that Smart Ticket compiles against:
/Users/Shared/smart_ticket2.0]% find . -name \*.zip -o -name \*.jar
./smart_ticket-client.jar
./src/tools/ant/lib/ant.jar
./src/tools/ant/lib/crimson.jar
./src/tools/ant/lib/jaxp.jar
Based on this output we configure the Lint4j Ant task as follows:
<lint4j packages="com.sun.j2me.*">
<sourcepath>
<dirset dir="${ticket.src.path}">
<include name="**/src" />
</dirset>
</sourcepath>
<classpath>
<fileset dir="${adventure.src.path}">
<include name="**/*.jar" />
</fileset>
</classpath>
</lint4j>
After the first run Lint4j reports missing classes that reduce the accuracy of the report, so the class path is augmented as follows:
<classpath>
<fileset dir="${adventure.src.path}">
<include name="**/*.jar" />
</fileset>
<pathelement path="${servlet.jar.path}" />
<pathelement path="${ejb.jar.path}" />
<pathelement path="${jms.jar.path}" />
<pathelement path="${mail.jar.path}" />
</classpath>
Now we are ready to perform the audit.
First, let's have a look at the most severe problems that Lint4j reports for Smart Ticket 2.0.
ant -Dlint4j.level=1 -emacs check-ticket Buildfile: build.xml check-ticket: src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/MovieRating.java:151: (1): Statement has no effect, possible scoping problem src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/TheaterSchedule.java:118: (1): The local variable "showTimes" shadows an accessible field with the same name and compatible type in class com.sun.j2me.blueprints.smartticket.shared.midp.model.TheaterSchedule.MovieSchedule src/app/server/ejb/src/com/sun/j2me/blueprints/smartticket/server/ejb/SmartTicketFacadeBean.java:234: (1): The local variable "account" shadows an accessible field with the same name and compatible type in class com.sun.j2me.blueprints.smartticket.server.ejb.SmartTicketFacadeBean src/app/server/ejb/src/com/sun/j2me/blueprints/smartticket/server/ejb/SmartTicketFacadeBean.java:451: (1): The local variable "account" shadows an accessible field with the same name and compatible type in class com.sun.j2me.blueprints.smartticket.server.ejb.SmartTicketFacadeBean src/app/server/ejb/src/com/sun/j2me/blueprints/smartticket/server/ejb/rating/MovieRatingData.java:128: (1): Statement has no effect, possible scoping problem src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/AccountPopulator.java:174: (1): The local variable "account" shadows an accessible field with the same name and compatible type in class com.sun.j2me.blueprints.smartticket.server.web.admin.populate.AccountPopulator src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/MoviePopulator.java:153: (1): The local variable "movie" shadows an accessible field with the same name and compatible type in class com.sun.j2me.blueprints.smartticket.server.web.admin.populate.MoviePopulator src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/TheaterPopulator.java:163: (1): The local variable "theater" shadows an accessible field with the same name and compatible type in class com.sun.j2me.blueprints.smartticket.server.web.admin.populate.TheaterPopulator src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/TheaterSchedulePopulator.java:115: (1): The local variable "theaterSchedule" shadows an accessible field with the same name and compatible type in class com.sun.j2me.blueprints.smartticket.server.web.admin.populate.TheaterSchedulePopulator src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/LocalModel.java:68: (1): Statement has no effect, possible scoping problem
Looking at the source code for the three scoping problems that were reported, the problems become obvious:
| actual code | correct code |
public void setLastViewingDate(long viewingDate) {
this.lastViewingDate = lastViewingDate;
[...]
}
|
public void setLastViewingDate(long viewingDate) {
this.lastViewingDate = viewingDate;
[...]
}
|
protected static ProgressObserver progressObserver;
public static void setProgressObserver(ProgressObserver progressObserver) {
progressObserver = progressObserver;
}
|
protected static ProgressObserver progressObserver;
public static void setProgressObserver(ProgressObserver progressObserver) {
this.progressObserver = progressObserver;
}
|
public void setLastViewingDate(long viewingDate) {
this.lastViewingDate = lastViewingDate;
[...]
}
|
public void setLastViewingDate(long viewingDate) {
this.lastViewingDate = viewingDate;
[...]
}
|
Next are several cases where a local variable shadows a field. The implementation appears to be correct, but it is confusing to a reader, which is not a good idea for any application, especially not for a sample application. An example of this kind of problem is below.
private AccountLocal account = null;
[..]
if (account == null ||!account.getUserName().equals(userName)
||!account.getPassword().equals(password)) {
try {
AccountLocal account = (AccountLocal) accountHome.findByPrimaryKey(userName);
if (account.getPassword().equals(password)) {
this.account = account;
Next comes warning level 3, since no issues are reported for this application for level 2.
ant -Dlint4j.level=3 -Dlint4j.exact=true -emacs check-ticket Buildfile: build.xml check-ticket: src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/Movie.java:51: (3): The method "equals" is overriden, but not the method "hashCode" src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/MovieRating.java:52: (3): The method "equals" is overriden, but not the method "hashCode" src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/RecommendationRecipient.java:52: (3): The method "equals" is overriden, but not the method "hashCode" src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/Theater.java:51: (3): The method "equals" is overriden, but not the method "hashCode" src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/midp/SmartTicketBD.java:99: (3): This catch block silently ignores the exception "javax.ejb.RemoveException". src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/midp/SmartTicketServlet.java:88: (3): This finally statement is empty and should be removed because it prevents the JIT from optimizing the code block. src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/midp/SmartTicketSessionListener.java:64: (3): This catch block silently ignores the exception "com.sun.j2me.blueprints.smartticket.shared.midp.ApplicationException". src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/PopulateServlet.java:178: (3): This catch block silently ignores the exception "com.sun.j2me.blueprints.smartticket.server.web.admin.populate.PopulateException". src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/PopulateServlet.java:228: (3): The serialization specification strongly discourages non-static serializable inner classes such as "com.sun.j2me.blueprints.smartticket.server.web.admin.populate.PopulateServlet.ParsingDoneException" src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/XMLDBHandler.java:89: (3): The method StringBuffer.setLength() should be avoided in favor of creating a new StringBuffer. src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/XMLDBHandler.java:167: (3): The method StringBuffer.setLength() should be avoided in favor of creating a new StringBuffer. src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/XMLDBHandler.java:189: (3): The method StringBuffer.setLength() should be avoided in favor of creating a new StringBuffer. src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/XMLDBHandler.java:278: (3): This catch block silently ignores the exception "java.lang.NumberFormatException". src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/LocalModel.java:308: (3): This catch block silently ignores the exception "com.sun.j2me.blueprints.smartticket.shared.midp.ApplicationException". src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/MessageHandler.java:53: (3): This class is declared abstract but does not have an abstract method. src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/RemoteModelRequestHandler.java:53: (3): This class is declared abstract but does not have an abstract method.
The four cases of a missing definition of the hashCode method seem to be harmless in the scope of this application, because these classes are not used as a key in a Map.
The empty finally block detected in SmartTicketServlet shows that something was supposed to happen here...
finally {
// FIXME
}
as is the case for the ignored exception in SmartTicketSessionListener
} catch (ApplicationException ae) {
// XXX
}
... where we also detect that the body of the method sessionDestroyed was commented out completely.
Next comes warning level 4.
ant -Dlint4j.level=4 -Dlint4j.exact=true -emacs check-ticket
Buildfile: build.xml
check-ticket:
[..]
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/IndexedResourceBundle.java:161: (4): The type "java.util.Vector" should be avoided and replaced with its counterpart from the Collections classes if possible.
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/IndexedResourceBundle.java:171: (4): Dont hardcode newline characters, use System.getProperty("line.separator") instead.
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/IndexedResourceBundle.java:171: (4): Dont hardcode newline characters, use System.getProperty("line.separator") instead.
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/IndexedResourceBundle.java:250: (4): It is strongly recommended to create a StringBuffer with a reasonable initial size instead of the default size of 16.
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/IndexedResourceBundle.java:252: (4): Dont hardcode newline characters, use System.getProperty("line.separator") instead.
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/IndexedResourceBundle.java:258: (4): Don't hardcode newline characters, use System.getProperty("line.separator") instead.
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/Movie.java:111: (4): Object types such as "com.sun.j2me.blueprints.smartticket.shared.midp.model.Movie" should be compared using the equals() method!
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/MovieRating.java:255: (4): Object types such as "com.sun.j2me.blueprints.smartticket.shared.midp.model.MovieRating" should be compared using the equals() method!
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/RecommendationRecipient.java:100: (4): Object types such as "com.sun.j2me.blueprints.smartticket.shared.midp.model.RecommendationRecipient" should be compared using the equals() method!
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/SeatingPlan.java:140: (4): It is strongly recommended to create a StringBuffer with a reasonable initial size instead of the default size of 16.
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/SeatingPlan.java:146: (4): Don't hardcode newline characters, use System.getProperty("line.separator") instead.
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/SyncAnchor.java:108: (4): It is strongly recommended to create a StringBuffer with a reasonable initial size instead of the default size of 16.
src/app/shared/src/com/sun/j2me/blueprints/smartticket/shared/midp/model/Theater.java:106: (4): Object types such as "com.sun.j2me.blueprints.smartticket.shared.midp.model.Theater" should be compared using the equals() method!
src/app/server/ejb/src/com/sun/j2me/blueprints/smartticket/server/ejb/SmartTicketFacadeBean.java:234: (4): The cast to type "com.sun.j2me.blueprints.smartticket.server.ejb.account.AccountLocal" is unnecessary, since the operand already is of the same type.
src/app/server/ejb/src/com/sun/j2me/blueprints/smartticket/server/ejb/SmartTicketFacadeBean.java:451: (4): The cast to type "com.sun.j2me.blueprints.smartticket.server.ejb.account.AccountLocal" is unnecessary, since the operand already is of the same type.
src/app/server/ejb/src/com/sun/j2me/blueprints/smartticket/server/ejb/show/Seating.java:174: (4): The cast to type "byte" is unnecessary, since the operand already is of the same type.
src/app/server/ejb/src/com/sun/j2me/blueprints/smartticket/server/ejb/show/Seating.java:179: (4): It is strongly recommended to create a StringBuffer with a reasonable initial size instead of the default size of 16.
src/app/server/ejb/src/com/sun/j2me/blueprints/smartticket/server/ejb/reservation/ReservationBean.java:120: (4): It is strongly recommended to create a StringBuffer with a reasonable initial size instead of the default size of 16.
src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/MovieSchedulePopulator.java:86: (4): It is strongly recommended to create a StringBuffer with a reasonable initial size instead of the default size of 16.
src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/MovieSchedulePopulator.java:97: (4): It is strongly recommended to create a StringBuffer with a reasonable initial size instead of the default size of 16.
src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/XMLDBHandler.java:59: (4): Make sure to null out the reference to type java.lang.StringBuffer as soon as possible to avoid holding on to too much memory or system resources
src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/XMLDBHandler.java:59: (4): It is strongly recommended to create a StringBuffer with a reasonable initial size instead of the default size of 16.
src/app/server/web/src/com/sun/j2me/blueprints/smartticket/server/web/admin/populate/XMLDBHandler.java:174: (4): This "if" statement is easier to understand if the negetion is eliminated, and the two branches are switched
src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/util/ApplicationUtilities.java:45: (4): It is strongly recommended to create a StringBuffer with a reasonable initial size instead of the default size of 16.
src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/util/ApplicationUtilities.java:179: (4): It is strongly recommended to create a StringBuffer with a reasonable initial size instead of the default size of 16.
src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/LocalModel.java:96: (4): The type "java.util.Vector" should be avoided and replaced with its counterpart from the Collections classes if possible.
src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/ModelFacade.java:357: (4): This "if" statement is easier to understand if the negetion is eliminated, and the two branches are switched
src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/ModelFacade.java:417: (4): It is strongly recommended to create a StringBuffer with a reasonable initial size instead of the default size of 16.
src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/Preferences.java:72: (4): The type "java.util.Hashtable" should be avoided and replaced with its counterpart from the Collections classes if possible.
src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/RemoteModelProxy.java:64: (4): The type "java.util.Hashtable" should be avoided and replaced with its counterpart from the Collections classes if possible.
src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/SynchronizationAgent.java:83: (4): The type "java.util.Vector" should be avoided and replaced with its counterpart from the Collections classes if possible.
src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/SynchronizationAgent.java:102: (4): The type "java.util.Hashtable" should be avoided and replaced with its counterpart from the Collections classes if possible.
src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/SynchronizationAgent.java:141: (4): The type "java.util.Hashtable" should be avoided and replaced with its counterpart from the Collections classes if possible.
src/app/client/midp/src/com/sun/j2me/blueprints/smartticket/client/midp/model/SynchronizationAgent.java:181: (4): The type "java.util.Hashtable" should be avoided and replaced with its counterpart from the Collections classes if possible.
At this warning level 114 unnecessary return statements at the end of a void method were logged, which increase code size for no reason. In addation, 20 debug statements (System.err.println and printStackTrace) were detected. These warnings are omitted in the listing above because of large number of occurences.
There are 8 cases where Vector and Hastable, both synchronized objects, could be replaced with the unsynchronized collections ArrayList and HashMap.
This concludes our quick code review of the Smart Ticket application.
