Wednesday, April 07, 2010

Why tabs are better

I'm tired of this stupid "tabs vs. spaces" code style debate. Tabs win hands down on just about every measure. Anyone still laboring under the misapprehension that it makes sense to indent one's source file using spaces should consider the following:
  1. Line-based comments (‘//’, ‘#’) at the head of the line don’t screw up the indentation (unless tab depth <= 2).
  2. You can change the indentation depth without editing the file. This is a huge feature, folks. If I like shallow indentation on all my source, I can make it so, and people who prefer the other extreme are not affected. The counter-argument (put forth by Checkstyle, among others) that one should not be required to set tab depth in order to read source is absurd; tab depth is always set to something, whether you like it or not (see 11), and code indented using tabs is readable regardless of the setting, unless tab depth is ridiculously high. The only code that actually does require a fixed tab depth to be legible is code that mixes tabs and spaces, which I encounter all too often. See 10.
  3. Spaces-based indentation will inevitably become inconsistent because no one can agree on his/her favorite indentation depth (see 2).
  4. Indentation mistakes are more obvious using tabs (unless tab depth = 1, which is just stupid).
  5. Tab indentation characters, when used properly, are more semantically relevant than spaces.
  6. Files are smaller (relevant especially for Javascript, CSS, HTML).
  7. Fewer keystrokes are needed to navigate within source files. Sorry, but “Ctrl+Right arrow” is two keystrokes, plus you have to hold one of them down.
  8. Making tabbed whitespace visible in an IDE is useful for eyeballing how things line up; making spaces visible is useful for “magic eye”.
  9. Tabs are unable to support the unreadable, but nevertheless default, function-call line-break style of making parameters line up with the opening ‘(’. Remember, it is a feature that this abomination is not supportable. Unfortunately it is still possible to put just the first parameter on the same line as the ‘(’, but no indentation choice can prevent that bad decision.
  10. If you have to edit a production config file using terminal-based default emacs, should you really be checking that in? I should add that the indentation used by default in Emacs (and pervasive in high-profile source such as the JDK) is a horrific hybrid of spaces and tabs which actually does force you to set your tab depth to a fixed value of 8 in order to read code thus indented. See 2.
  11. Some well-known tools (e.g., ReviewBoard) typically display tabs with a depth of 8, which is kind of high. I claim that this tab discrimination is also a feature, because it discourages deeply-nested code which is a good thing.

The only moderately sane argument in favor of spaces is that the code "always looks the same". Isn't that nice. You can write comments that use little "^^^^" to point to something on the line above. Wow. I guess that's worth throwing out points 1-11.

I'm not going to wade into the quagmire of my other personal code style choices. But it's time this debate, which rages again and again every time I join a new team, be permanently put to bed.

Thursday, June 25, 2009

JAXB, @XmlMixed, and white space anomalies

Whether or not you think "mixed" content in XML is ever a good idea, you may need to handle it using JAXB one day. Recall that for JAXB to parse a mixed content XML element to a class C, you use an @XmlMixed annotation on a field of C of type List< Serializable >, combined with either @XmlAnyElement or @XmlElements. In each case, the resulting list will contain Strings representing the text nodes and objects representing the element nodes, in the same order as they appear in the XML text. Thus

<thing>stuff<nested/>entities<alsoNested/></thing>
maps to an instance of
@XmlRootElement
class Thing {
    @XmlMixed @XmlAnyElement
    List< Serializable > lserComponents;
}
which looks like
{ lserComponents : [ "stuff", { localName : "nested" }, "entities", { localName: "alsoNested" } ] }
Unfortunately, if the only content other than nested elements happens to be white space, as in
<thing><nested/>   <alsoNested/></thing>
you get the odd bound object
{ lserComponents : [ { localName : "nested" }, { localName: "alsoNested" }, "" ] }
If you care about white space, and who doesn't these days in the throes of late-stage Reaganomics, you need a trick when you actually go to parse the XML.

First, we create a SAX 2.0 ContentHandler implementation that delegates all events to a JAXB UnmarshallerHandler, but modifies all the whitespace slightly:

class WhitespaceAwareUnmarshallerHandler implements ContentHandler {
  private final UnmarshallerHandler uh;
  public WhitespaceAwareUnmarshallerHandler( UnmarshallerHandler uh ) {
    this.uh = uh;
  }
  /**
   * Replace all-whitespace character blocks with the character '\u000B',
   * which satisfies the following properties:
   * 
   * 1. "\u000B".matches( "\\s" ) == true
   * 2. when parsing XmlMixed content, JAXB does not suppress the whitespace
   **/
  public void characters(
    char[] ch, int start, int length
  ) throws SAXException {
    for ( int i = start + length - 1; i >= start; --i )
      if ( !Character.isWhitespace( ch[ i ] ) ) {
        uh.characters( ch, start, length );
        return;
      }
    Arrays.fill( ch, start, start + length, '\u000B' );
    uh.characters( ch, start, length );
  }
  /* what follows is just blind delegation monkey code */
  public void ignorableWhitespace( char[] ch, int start, int length ) throws SAXException { uh.characters( ch, start, length ); }
  public void endDocument() throws SAXException { uh.endDocument(); }
  public void endElement( String uri, String localName, String name ) throws SAXException { uh.endElement( uri,  localName, name ); }
  public void endPrefixMapping( String prefix ) throws SAXException { uh.endPrefixMapping( prefix ); }
  public void processingInstruction( String target, String data ) throws SAXException { uh.processingInstruction(  target, data ); }
  public void setDocumentLocator( Locator locator ) { uh.setDocumentLocator( locator ); }
  public void skippedEntity( String name ) throws SAXException { uh.skippedEntity( name ); }
  public void startDocument() throws SAXException { uh.startDocument(); }
  public void startElement( String uri, String localName, String name, Attributes atts ) throws SAXException { uh.startElement( uri, localName, name, atts ); }
  public void startPrefixMapping( String prefix, String uri ) throws SAXException { uh.startPrefixMapping( prefix, uri ); }
}
Then at parse time, instead of the usual ctx.createUnmarhaller().unmarshal( strData ), we substitute our special handler to do the parsing:
public class JAXBUtil {
  @SuppressWarnings( "unchecked" )
  public static < T > T unmarshal(
    JAXBContext ctx, String strData, boolean flgWhitespaceAware
  ) throws Exception {
    UnmarshallerHandler uh = ctx.createUnmarshaller().getUnmarshallerHandler();
    XMLReader xr = new WstxSAXParser(); // use your favorite SAX 2.0 parser
    xr.setContentHandler( flgWhitespaceAware ? new WhitespaceAwareUnmarshallerHandler( uh ) : uh );
    xr.parse( new InputSource( new StringReader( strData ) ) );
    return ( T )uh.getResult();
  }
}

Thursday, May 15, 2008

Identity transformation, my butt

Some lovely trivia I have recently discovered about the default implementations of XSLT transformations in the JDK 1.5:


  1. The so-called "identity transformation" available at TransformerFactory.newTransformer() is anything but the identity when applied to XHTML, until certain non-default configuration is applied. Specifically, you have to do all this:


    xfmEng.setOutputProperty( OutputKeys.DOCTYPE_SYSTEM, "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd" );
    xfmEng.setOutputProperty( OutputKeys.DOCTYPE_PUBLIC, "-//W3C//DTD XHTML 1.0 Transitional//EN" );
    xfmEng.setOutputProperty( OutputKeys.METHOD, "html" );
    xfmEng.setOutputProperty( OutputKeys.OMIT_XML_DECLARATION, "yes" );


    or you get tons of <!-- ... --> garbage before the real document. The garbage seems to live in the w3c.org dtd files for xhtml.

  2. Even with all that, you still end up with the very non-identity transformation of input like <script src=...></script> becoming <script src=.../>. The latter is actually malformed according to many browsers. Forget newTransformer() and use an xslt-based transformation like

    <?xml version="1.0" encoding="UTF-8"?>
    <xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
    <xsl:output method="html"/>
    <xsl:template match="/">
    <xsl:copy-of select="."/>
    </xsl:template>
    </xsl:stylesheet>

  3. Speaking of the w3c.org dtd files, there's still some nasty stuff going on behind the scenes; when any transformer created via TransformerFactory.newTransformer() or Templates.newTransformer() starts processing XHTML, it actually goes and grabs those extremely well-known DTDs off the web from their URIs at w3c.org. Every document, even with the same transformer, engenders a new set of GETs to w3c. Pretty ridiculous. Here's how to get around that:


    package MyPackage;

    import org.xml.sax.SAXNotRecognizedException;
    import org.xml.sax.SAXNotSupportedException;
    import com.sun.org.apache.xerces.internal.impl.Constants;
    import com.sun.org.apache.xerces.internal.parsers.SAXParser;

    public class MyTransform {

    // ...

    public static class MySAXParser extends SAXParser {
    public MySAXParser() {
    super();
    try {
    setFeature( Constants.SAX_FEATURE_PREFIX + Constants.VALIDATION_FEATURE, false );
    setFeature( Constants.XERCES_FEATURE_PREFIX + Constants.LOAD_EXTERNAL_DTD_FEATURE, false );
    } catch ( SAXNotRecognizedException sne ) {
    } catch ( SAXNotSupportedException sse ) {
    }
    }
    }

    // in the code that uses the transformer:
    System.setProperty( "org.xml.sax.driver", "MyPackage.MyTransform$MySAXParser" );
    TransformerFactory.newInstance().newTransformer().transform( stmIn, stmOut );

    // ...
    }


AspectJ: A crucial distinction between execution and call join points

Given the following code:


class Super {
protected void m() {}
}
class Sub extends Super {}


A call( * Sub+.m( .. ) ) join point will match any call to m made on an instance statically known to be a Sub (in particular, ( ( Super )s ).m() will not only not match, but will cause a compiler warning); an execution( * Sub+.m() ) join point will not match s.m(), even if s is Sub, because there's no m() code to execute in Sub!

Spring declarative transactions, applied dynamically

I think declarative transaction management is possibly the most compelling reason for using the Spring Framework. I won't explain how it usually works, leaving that to the Spring docs and plenty of examples in the literature. Recently I ran into the following conundrum, however.

I have a service class that has not only some "transactional" methods, but also other methods that return instances of secondary classes with their own transactional methods. It is easy in the configuration of the Spring container to declare the transactional methods in the service as such, because it is a typical singleton bean. The problem is that if the code in the service implementation is to remain decoupled from Spring, how do we instruct Spring to wrap transactional proxies around any instances of the secondary classes that are produced at runtime?

My first attempt to solve this problem was to factor the code that needed to be transactional out of the secondary classes into package-access methods on the main service, in the hopes that I could declare those helper methods transactional in the configuration. Then the formerly transactional methods in the secondary classes could call those helper methods back in the service bean. This attempt proved to be not just inelegant, but also ineffectual: a complete waste of time.

Why didn't it work? Because the newly-minted secondary class instances were not receiving a reference to the service bean's transactional proxy to make their call backs on; they got the unadorned service instance as obtained via this, and in order to get the proxy, the Spring coupling would have to creep back into the code. Only a more complete AOP framework like AspectJ, with its special compiler, could avoid the this problem.

My solution to this problem was in four parts. Spring 2.5 is required.


  1. Create a new implementation of each secondary class which is a pure delegating proxy. The constructor of DelegatingSecondaryClassImpl takes a SecondaryClass as delegate, and every method simply forwards to that delegate.

  2. Create a BeanFactoryAware aspect ServiceAdvice with around advice for the methods on the service class that produce instances of these secondary classes, one advice method per secondary class. In each case the advice looks like this:

    public SecondaryClass secondaryClassAroundAdvice( ProceedingJoinPoint pjp ) throws Throwable {
    return ( SecondaryClass )bf.getBean( "secondaryClassBeanId", new Object[] { pjp.proceed() } );
    }


  3. Add a prototype bean to the configuration XML for each secondary class:

    <bean id="secondaryClassBeanId" class="com.amazon.foo.bar.DelegatingSecondaryClassImpl" scope="prototype">
    <constructor-arg><null/></constructor-arg>
    </bean>


  4. Add advice to the service methods that produce instances of each secondary class in the configuration XML:

    <bean id="serviceAdvice" class="com.amazon.foo.bar.ServiceAdvice">
    <aop:config>
    <!-- transactional advisor goes here ... -->
    <aop:aspect id="proxyAspect" ref="proxyAdvice">
    <aop:around pointcut="execution( com.amazon.foo.bar.SecondaryClass createSecondaryClass1( .. ) ) || execution( com.amazon.foo.bar.SecondaryClass createSecondaryClass2( .. ) )" method="secondaryClassAroundAdvice">
    </aop:around>
    </aop:aspect>
    </aop:config>


Friday, August 03, 2007

AspectJ for lazy loading, improved

I find Russ Miles' lazy feature loading recipe (AspectJ Cookbook, chapter 21.3) cumbersome and at odds with the whole point of AOP. You have to use joinPoint.getArgs inside a proxy; but you're using AspectJ instead of Spring to avoid the proxy-based Spring AOP implementation, with all its intrinsic problems (what does "this" mean?). You also have to be aware in the mainline code that you're doing lazy loading and explicitly call LazyLoading.aspectOf().initializeFeature(), so it's intrusive.

In addition, there is essentially no reusability here. You have to create a special aspect for each interface which you wish to load lazily, with stubs for every method of that interface.

Finally, you have to use a weird trick of declaring that one interface implements another without actually giving any implementation (this line is given with no explanation in the text).

My approach, by contrast, is extremely simple. I do still impose a burden on the mainline code; specifically, if you want to see any benefit from lazy loading of an object you have to make sure the expensive initialization code actually occurs within the construction of that object. But that's it. No proxies, no hard-coded interface stubs, no intrusion.

The basic technique is to suppress the execution of the object's initialization code, wrapping it in a closure and storing it away until the first access to the object that requires that the initialization take place. The storage of the closure, and the state of the flag indicating that the initialization has or has not been attempted, live inside a small pertarget aspect.

Here's two variations of the LazyInitialization aspect.

The first:

// one aspect instance per lazily-instantiated object
public aspect LazyInitialization pertarget( target( Lazy ) ) {

// this interface represents a closure for the real initialization
private interface DelayedInit {
 void init();
}

// this is the marker interface for types that should be
// lazily initialized
public interface Lazy {};

 // per-object aspect state
 boolean flgTried = false;
 DelayedInit fnInit = null;

 // this advice intercepts the normal initialization process for
 // a Lazy object and stashes a closure for it in the aspect
 void around() : execution( Lazy+.new( .. ) ) {
  synchronized( this ) {
   if ( fnInit == null )
    fnInit = new DelayedInit() {
     public void init() {
      proceed();
     }
    };
  }
 }

  // this advice intercepts any access to a Lazy object that might
  // require it to actually initialize, and does so if necessary
  before() : call( * Lazy+.*( .. ) ) || get( * Lazy+.* ) || set( * Lazy+.* ) {
   synchronized( this ) {
    if ( !flgTried ) {
     flgTried = true;
     fnInit.init();
    }
   }
  }
}

To use it, just declare somewhere that some class or interface C implements LazyInitialization.Lazy and every instance of C or its subclasses/implementations will be lazily initialized:

 
aspect MakeCLazy {
   declare parents : C+ implements LazyInitialization.Lazy;
}

The other variation uses a generic type variable T instead of a marker interface. This version allows you to be more specific about which classes/interfaces/methods are to be lazily initialized and what constitutes an initialization-worthy access via filter pointcuts.

public abstract aspect GenericLazyInit< T > pertarget( initFilter() ) {

 // this interface represents a closure for the real initialization
 private interface DelayedInit {
  void init();
 }

 // per-object aspect state
 boolean flgTried = false;
 DelayedInit fnInit = null;

 protected pointcut initFilter() : execution( T+.new( .. ) );
 protected pointcut accessFilter() : call( * T+.*( .. ) ) || get( * T+.* ) || set( * T+.* );
 
 // this advice intercepts the normal initialization process for
 // a Lazy object and stashes a closure for it in the aspect
 void around() : initFilter() {
  synchronized( this ) {
   if ( fnInit == null )
    fnInit = new DelayedInit() {
     public void init() {
      proceed();
     }
    };
  }
 }

 // this advice intercepts any access to a Lazy object that might
 // require it to actually initialize, and does so if necessary
 before() : accessFilter() {
  synchronized( this ) {
   if ( !flgTried && fnInit != null ) {
    flgTried = true;
    fnInit.init();
   }
  }
 }
}

Using this version (in its default configuration, equivalent to variation 1) is even briefer:

 
aspect MakeCLazy extends GenericLazyInit< C > {}

Tuesday, October 10, 2006

But...but...we haven't been attacked again since 9/11!

May I remind you that we've now lost about as many soldiers in Iraq as we lost civilians. Iraqis have lost something like 8 to 20 times as many civilians. Terrorist attacks have skyrocketed since 9/11 worldwide. We weren't attacked for five years before 9/11 either on U.S. soil, unless you count the Oklahoma City bombing.

What, you want a smug response to this canard? OK, how about: that's like saying we didn't get robbed again since we started leaving stacks of $100 bills on the corner every night.

Any other good metaphors?