Code reviews (or code inspections) have been shown anecdotally and through research to be one of the best ways of discovering defects, getting developers on the same page, ensuring a consistent code style, and more. There are many ways to do it from pair programming to getting everyone in the room and putting code up on the projector. While it can be difficult to justify the time expense at first, code reviews quickly pay off.  More than once I’ve seen defects caught that could have caused tens of hours (and much more) in bug fixing later be caught at the outset. It’s also a great way for developers to keep each other sharp and improve their knowledge and skills.

But this post really isn’t about why you should do code reviews but rather a tip on how. If you Google around for Eclipse code review plugins you will find the typical mix of open source and commercial. However, given that code reviews can be done so many different ways I was unable to find one that resonated with me; they all assume an explicit process (usually somewhat formal) that is not the one I’m looking for. Also, with our variation in project and team sizes, some flexibility in the process is needed.

Continue reading…

In a component-based framework such as Tapestry 5 each component may contribute it’s own Javascript initialization for creating objects, form field initialization, etc. The result is a block of Javascript executing on page load similar to this:

Tapestry.onDOMLoaded(function() {
new PromptSaveOnDirtyForm()
$('form').observe('reset',function(){window.location.href='/example/projectdetail.traditionalpricinglayout.pricinglayout.form:reset?t:ac=1/bookspecspricing&t:cp=bookspecspricing'; return false;});
new DisplayFragmentSelect('impressionType','impressionhideable',0.1);
new BindingStyleUpdater('impressionType','bindingStyle');
new Ajax.Autocompleter('trimSize', 'trimSize:menu', '/example/projectdetail.trimsize:autocomplete?t:ac=1/bookspecspricing&t:cp=bookspecspricing', {"indicator":"trimSize:loader","paramName":"t:input"});
linkMediaFragment('bundledMedia','cdDvdWrapper');
BeanEditGrid.reorder('miscCosts_0:container');
BeanEditGrid.prepareDragDrop('miscCosts_0:container');
new JSONAutocomplete('name', 'name:menu', '/example/projectdetail.traditionalpricinglayout.name:autocomplete?t:ac=1/bookspecspricing&t:cp=bookspecspricing', $H({"indicator":"name:loader","paramName":"t:input"}).update({onSelect: updateCostType.curry('name')}).toObject());
new DisplayFragmentSelect('salesType','sthideable',0.1);
new DisplayFragmentSelect('businessType','bthideable',0.1);
new SelectOther('source','otherFragment','dummy');
new FireValueChanged('referenceISBN','/example/projectdetail.traditionalpricinglayout.printrundisplay.referenceisbn:fieldValueChanged?t:ac=1/bookspecspricing&t:cp=bookspecspricing',true,false,updateValues)
tabbox_tabbox = new TabBox(['miscCosts','printRun','itemMaster','notes'],'miscCosts','tabbox','selectedTabIndex');
if (typeof form_fireidupdated == 'undefined' ) {form_fireidupdated = new FireIdUpdated('form'); } else { form_fireidupdated.updateAndFire('form')}
new BookSpecsPricing('permissions','prePress','cover')
$('salesUnits').activate();
Tapestry.init({"zone":[{"update":"show","element":"formzone"},"historyZone"],"formLoopRemoveLink":[{"link":"removerowlink","fragment":"fragment","url":"/example/projectdetail.traditionalpricinglayout.projectcostnames:triggerRemoveRow/31340284_1268329414774/java.lang.String?t:ac=1/bookspecspricing&t:cp=bookspecspricing"},{"link":"removerowlink_0","fragment":"fragment_0","url":"/example/projectdetail.traditionalpricinglayout.projectcostnames:triggerRemoveRow/32235117_1268329414774/java.lang.String?t:ac=1/bookspecspricing&t:cp=bookspecspricing"} /* a LOT more javascript here */);

For a complex page, performing initialization for a large number of components can cause long page load times in the browser (especially IE). For instance, one particular page had a lightbox full of fields as well as a set of tabs with more fields. However, the initialization for these components never needs to occur if the lightbox is never loaded or the tabs never opened.

Our solution was to create a “lazy” component allowing any Javascript contributed during render to be wrapped in a closure function. This function will lazy-initialize it’s contents and can safely be called multiple times.

Using the component looks like this:

<div style="display:none;" id="specsWrapper" t:type="any" t:mixins="lazy" function="specsLazyInit">   
<!-- large amounts of components and lightbox code --> 
</div> 
<p><a href="#" onclick="specsLazyInit();">Show the lightbox!</a> 

The wrapper function is given a specific name that we can safely call before showing the UI. This would change the previous Javascript to something like this:

Tapestry.onDOMLoaded(function() {
$('form').observe('reset',function(){window.location.href='/example/projectdetail.traditionalpricinglayout.pricinglayout.form:reset?t:ac=1/bookspecspricing&t:cp=bookspecspricing'; return false;});

// init code for lightbox.
// note that it can be called multiple times safely and uses the function passed in as a parameter to the mixin
window.specsLazyInitState=false; window.specsLazyInit = function(){ if (window.specsLazyInitState) return; window.specsLazyInitState=true;
  new DisplayFragmentSelect('impressionType','impressionhideable',0.1);
  new BindingStyleUpdater('impressionType','bindingStyle');
  new Ajax.Autocompleter('trimSize', 'trimSize:menu', '/example/projectdetail.trimsize:autocomplete?t:ac=1/bookspecspricing&t:cp=bookspecspricing', {"indicator":"trimSize:loader","paramName":"t:input"});
  linkMediaFragment('bundledMedia','cdDvdWrapper');
  Tapestry.init({"formLoopRemoveLink":[{"link":"removerowlink","fragment":"fragment"}] /* more js */);
 };

// init code for some tabs
// if the function is empty we still create it so it's safe to always call
window._miscCostsLazyInit = function(){};
BeanEditGrid.reorder('miscCosts_0:container');
BeanEditGrid.prepareDragDrop('miscCosts_0:container');
window._printRunLazyInitState=false; window._printRunLazyInit = function(){ if (window._printRunLazyInitState) return; window._printRunLazyInitState=true;
  new DisplayFragmentSelect('salesType','sthideable',0.1);
  new DisplayFragmentSelect('businessType','bthideable',0.1);
  new SelectOther('source','otherFragment','dummy');
  new FireValueChanged('referenceISBN','/example/projectdetail.traditionalpricinglayout.printrundisplay.referenceisbn:fieldValueChanged?t:ac=1/bookspecspricing&t:cp=bookspecspricing',true,false,updateValues)
  Tapestry.init({"zone":["historyZone"]} /* more js */);
 };
window._itemMasterLazyInitState=false; window._itemMasterLazyInit = function(){ if (window._itemMasterLazyInitState) return; window._itemMasterLazyInitState=true;
  Tapestry.init( /* more js */);
 };
window._notesLazyInit = function(){};

tabbox_tabbox = new TabBox(['miscCosts','printRun','itemMaster','notes'],'miscCosts','tabbox','selectedTabIndex');
if (typeof form_fireidupdated == 'undefined' ) {form_fireidupdated = new FireIdUpdated('form'); } else { form_fireidupdated.updateAndFire('form')}
new BookSpecsPricing('permissions','prePress','cover');
postScriptInit();
$('salesUnits').activate();
Tapestry.init({"zone":[{"update":"show","element":"formzone"}],"linkZone":[["form","formzone"]],"formInjector":[{"element":"rowInjector_0","url":"/example/projectdetail.traditionalpricinglayout.misccosts.ajaxformloop.rowinjector:inject?edit-grid:id=miscCosts_0:container&t:ac=1/bookspecspricing&t:cp=bookspecspricing&t:formid=form"} /* more js here */);
});

Now all that code is parsed but not executed resulting in faster page loads and the ability to create complex pages while maintaining acceptable client-side performance. Even in Firefox the load time after the DOM had loaded went from 5-8s to 1s or less.

In a complex application you can have a number of JavaScript files loaded by the client including both 3rd-party and application-specific libraries. In one application this included prototype.js, scriptaculous.js (as well as effects.js, dragdrop.js, controls.js), window.js (for lightbox support), and application.js. For development purposes, keeping these separate is ideal but for production purposes, loading this many files can really slow down the browser even if the files are cached.

We created a method whereby all the core javascript libraries for the application are combined into a single, minified (and munged) library that can then be cached at first page load by clients. This uses YUI compressor, Maven 2, and the exec plugin.

Having added yuicompressor as a maven dependency we need to configure our code to run as part of the build:

<!-- generates the static corelibraries.js file -->
<plugin>
    <groupId>org.codehaus.mojo</groupId>
    <artifactId>exec-maven-plugin</artifactId>
    <version>1.1.1</version>
    <executions>
        <execution> 
            <id>corelibraries</id>
            <phase>process-classes</phase>
            <goals><goal>java</goal></goals>
            <configuration>
                <mainClass>com.ifactory.example.GenerateCoreLibraries</mainClass>
            </configuration> 
        </execution>
     </executions>
</plugin>

Now we need our class to generate the file:

/** Runs as part of the build to generate the corelibraries.js file. */
public final class GenerateCoreLibraries {
    public static void main(String args[]) throws Exception {
    List files = Arrays.asList(
        "file:src/main/webapp/js/scriptaculous/prototype.js",
        "file:src/main/webapp/js/scriptaculous/scriptaculous.js",
        "file:src/main/webapp/js/scriptaculous/effects.js",
        "file:src/main/webapp/js/scriptaculous/controls.js",
        "file:src/main/webapp/js/scriptaculous/dragdrop.js",
        "classpath:/org/apache/tapestry5/tapestry.js",
        "file:src/main/webapp/js/window.js",
        "classpath:/com/ifactory/example/application.js"
        );

    File outFile = new File("src/main/webapp/js/corelibraries.js");

    if (outFile.exists()) {
      System.out.println("Deleting " + outFile + " to ensure it's fresh");
      if (!outFile.delete())
        throw new RuntimeException(outFile + " could not be deleted");
    }

    System.out.println("Generating " + outFile + " from " + StringUtils.join(files, ", "));
    StringBuilder b = new StringBuilder();
    for(String file : files) {
      InputStream is;

      if (file.startsWith("file:"))
        is = new FileInputStream(new File(file.replaceFirst("file:", "")));
      else if (file.startsWith("classpath:")) {
        String path = file.replaceFirst("classpath:/?", "");  // root / must be removed
        is = GenerateCoreLibraries.class.getClassLoader().getResourceAsStream(path);
        if (is == null)
          throw new IllegalArgumentException(path + " not found");
      }
      else
        throw new UnsupportedOperationException(file);

      try {
        b.append("// BEGIN ").append(file).append("\n");
        b.append(IOUtils.toString(is)).append("\n");
      } finally {
        IOUtils.closeQuietly(is);
      }
    }

    ErrorReporter reporter = new ErrorReporter() {
      public void warning(String message, String sourceName, int line, String lineSource, int lineOffset) {}

      public EvaluatorException runtimeError(String message, String sourceName, int line, String lineSource, int lineOffset) {
        throw new RuntimeException(String.format("%s line %d %s", sourceName, line, message));
      }

      public void error(String message, String sourceName, int line, String lineSource, int lineOffset) {
        System.err.println(String.format("%s line %d %s", new Object[] {sourceName, line, message}));
      }
    };

    JavaScriptCompressor compressor = new JavaScriptCompressor(new StringReader(b.toString()), reporter);

    ByteArrayOutputStream out = new ByteArrayOutputStream();
    Writer writer = new OutputStreamWriter(out);
    compressor.compress(writer, -1, true, false, true, false);
    writer.flush();
    writer.close();

    FileUtils.writeStringToFile(outFile, out.toString());
  }
}

When running a local Jetty for development or when packaging the WAR this class will get run. An additional advantage of this approach is that the generated file is static and so can be easily cached & compressed by the container.