Child pages
  • Sakai Programming Best Practices
Skip to end of metadata
Go to start of metadata
  • Documents and defines best practices for programming in Sakai (though many of these are generally good pratices to follow for Java in general).

Minimal usage of synchronized collections (Hashtable, Vector, etc.)

The use of synchronized collections (Vector, Hashtable) should be a last resort and only used when absolutely necessary. These should primarily be used for thread safety. However, the cost of writes AND reads are expensive when using these (easily 10-100x more than using the basic unsychronized versions). The cost of synchronization can become even more severe in a highly concurrent environment like Sakai, especially when it is deployed on a multi-core server.
When thread safety is not needed (99% of the time in Sakai) an ArrayList, HashSet, or HashMap should be used instead.
If thread safety is needed together with the performance of lock-free code, the best choice will usually be ConcurrentHashMap. CopyOnWriteArrayList and CopyOnWriteArraySet are other classes that could be useful in very specialised situations.
More info on concurrent collections here:

Use StringBuilder for appending strings when appropriate

Strings can be appended in many ways in java. The most common are to simply add the strings together, use StringBuilder, or use StringBuffer. These are all appropriate at different times.

  1. Adding Strings
    This is appropriate when all the strings are appended in a single statement.
    String newVal = string1 + " " + string2 + " more stuff " + blah.toString() + ":" + thing.getValue().title;
    You might hear people telling you that you should be using a StringBuffer/StringBuilder here but that is silly because that is what the compiler converts this into anyway. No reason to make your code longer and uglier.
  2. Using StringBuilder
    This is appropriate when you are appending strings in multiple statements. I would personally not bother unless there are 3+ strings to append.
    StringBuilder sb = new StringBuilder();
    for (int i=0; i < thing.size(); i++) {
    String newVal = sb.toString();
  3. Using StringBuffer
    In Java 1.5-level code and above, any explicit use of StringBuffer should be replaced with the lock-free equivalent StringBuilder. There is almost never a good reason to use StringBuffer. This is a synchronized object and therefore very slow to use unless you actually need the synchronization. In general, you should never use this over StringBuilder unless you have a very good reason (and need thread safety).

Minimize API dependencies

Interfaces in Sakai services (and in general) should be written to minimize dependencies on other packages when possible. This means not requiring a User object when the identity of a user is needed, but instead requiring the userID. When using an id instead of the actual object you should clearly identify what is expected in the javadocs like so:

    * @param assignGroupId the id of an {@link EvalAssignGroup} object to remove
    * @param userId the internal user id (not username or eid)
   public void deleteAssignGroup(Long assignGroupId, String userId);

Use numeric autogenerated ids in database tables

There are many ways to generate IDs for database tables, but one of the simplest, fastest, and safest is to allow the database to do it for you using numeric increamenting. This is supported in every database and is easy to do with hibernate. This also can reduce locking issues in MySQL and creates relatively simple looking IDs. Here are some helpful links:
HSQLDB Identity MYSQL autoincrement ORACLE sequence

  • If you want to ensure that you also have globally unique ids for your data, there are 2 ways to make these globally safe
    1. Append prefixes like: edu.rutgers.sakai.evaluation.answer.123, (system-prefix).(app-prefix).(table-prefix).id
    2. Store an eid (GUID, or whatever you like) in the tables along with the autogened id and use that externally

Compare strings/constants by putting them on the left side of the comparison

You can avoid many NullPointerExceptions by simply always placing hardcoded string and constants on the left side of an object comparison. This works well for any object constants (not just strings).
For example, this code can generate a NullPointerException if myString is null (bad):

if (myString.equals("hardcodedstring")) {
    // do something

This example code can never generate a NullPointerException (good):

if ("hardcodedstring".equals(myString)) {
    // do something

Cleanup user submitted strings before storing them (i.e. do not trust input)

You can avoid issues related to XSS (Cross Site Scripting) by simply cleaning up the strings that are submitted to your services. This should be done in all systems and can be handled in the webapp (tool) or in the services (probably more ideal). In Sakai this can be done using the utils from the kernel but external jars are fine as well if you have one you are comfortable with.
For example, this code takes a user submitted string and returns the cleaned up version. It would be called any time there is any user data being input (that includes data from web form radio buttons and pulldowns).

String cleaned = FormattedText.processFormattedText(userSubmittedString, new StringBuffer());

Ideally a method that does not require creating a StringBuffer would be available but that is not currently the case.

  • No labels


  1. Concerning minimal API dependencies – I agree completely. However, I would strongly suggested that all identifiers (etc) passed as Strings be completely documented everywhere they are used. There are far too many places with API's of the form:

        public void myMethod (String id);

    With no clue as to what kind of id it is. The parameter name should always document the kind of id it is along with Javadoc:

         *  @parameter userId A valid Sakai user id (GUID, not EID) that will be used to ....
        public void myMethod (String userId);
  2. The processFormattedText example should be updated to be the StringBuilder method, as the StringBuffer method is deprecated, at least in the 2.5.2 Javadoc. Perhaps a note saying that this parameter holds the error messages returned so people can use it in the UI?

    1. That's part of the point here. A string returned from a static util should never be used in the UI because it is not i18n. If this were throwing exceptions or something which indicated the failure then the UI could interpret the codes in the exception into real messages but until that is the case I am not sure that this is really all that valuable. For code that is not meant to work on older versions of Sakai it is fine to use the StringBuffer version I suppose though it would be a shame to lose compatibility just for a change like that.

      1. Ah I see what you are getting at, I wasn't thinking in i18n terms. A way around this would be if the tools returned a standard set of strings that could then be translated in the UI,

        ie "error.uploadtoolarge"

        rather than the full text "The file you uploaded was too large". That could then be internationalised in the tool perhaps.