Various "stylistic" improvements suggested by Joshua Bloch's book.

Use @Override annotation.  Limit accessibility of fields.  Make final
what can be.  Use complex enums.  Don't ignore exceptions.  Some more
changes on exceptions thrown.

14 files changed, 79 insertions, 43 deletions

diff --git a/org/madore/damlengine/DamlEngine.java b/org/madore/damlengine/DamlEngine.java
index c9bd477..2cde6ec 100644
--- a/org/madore/damlengine/DamlEngine.java
+++ b/org/madore/damlengine/DamlEngine.java
@@ -13,7 +13,9 @@ public final class DamlEngine {
     public static final String XHTML_NS = "http://www.w3.org/1999/xhtml";
     public static final String DAML_NS = "http://www.madore.org/~david/NS/daml/";
 
-    private DamlEngine() { }  // Forbid instantiation
+    private DamlEngine() { // Forbid instantiation
+	throw new AssertionError("DamlEngine cannot be instantiated");
+    }
 
     public static class RootTodo extends TodoItem {
 	public RootTodo(Context ctx) {
diff --git a/org/madore/damlengine/LangHelper.java b/org/madore/damlengine/LangHelper.java
index c1d736c..a94f5be 100644
--- a/org/madore/damlengine/LangHelper.java
+++ b/org/madore/damlengine/LangHelper.java
@@ -24,7 +24,7 @@ public class LangHelper {
 
     public static void setLangNorec(Element node, String lang) {
 	if ( lang == null )
-	    throw new IllegalArgumentException("lang is null in setLangNorec");
+	    throw new NullPointerException("lang is null in setLangNorec");
 	node.setAttributeNS(DamlEngine.XML_NS, LANG, lang);
     }
 
@@ -46,7 +46,7 @@ public class LangHelper {
 
     public static void setLangRec(Element node, String lang) {
 	if ( lang == null )
-	    throw new IllegalArgumentException("lang is null in setLangRec");
+	    throw new NullPointerException("lang is null in setLangRec");
 	String parentLang = getLangRec(node.getParentNode());
 	if ( lang.equals(parentLang) )
 	    unsetLangNorec(node);
diff --git a/org/madore/damlengine/Resolver.java b/org/madore/damlengine/Resolver.java
index 103659c..e04d1b2 100644
--- a/org/madore/damlengine/Resolver.java
+++ b/org/madore/damlengine/Resolver.java
@@ -29,6 +29,7 @@ public final class Resolver
 
     public static final class BootstrapResolver
 	extends org.apache.xml.resolver.helpers.BootstrapResolver {
+	@Override
 	public InputSource resolveEntity(String publicId, String systemId) {
 	    String resolved = null;
 	    if ( publicId.equals("-//GlobalTransCorp//DTD XML Catalogs "
@@ -65,7 +66,9 @@ public final class Resolver
 	    try {
 		catalog.parseCatalog(f);
 	    } catch (MalformedURLException e) {
+		throw new RuntimeException(e);
 	    } catch (IOException e) {
+		throw new RuntimeException(e);
 	    }
 	}
     }
@@ -74,9 +77,9 @@ public final class Resolver
 	try {
 	    return catalog.resolvePublic(publicId, systemId);
 	} catch (MalformedURLException e) {
-	    return null;
+	    throw new RuntimeException(e);
 	} catch (IOException e) {
-	    return null;
+	    throw new RuntimeException(e);
 	}
     }
 
@@ -84,9 +87,9 @@ public final class Resolver
 	try {
 	    return catalog.resolveSystem(systemId);
 	} catch (MalformedURLException e) {
-	    return null;
+	    throw new RuntimeException(e);
 	} catch (IOException e) {
-	    return null;
+	    throw new RuntimeException(e);
 	}
     }
 
@@ -107,6 +110,7 @@ public final class Resolver
 		URI v = u.resolve(systemId);
 		return v.toString();
 	    } catch ( URISyntaxException e ) {
+		// Fail and continue to next return.
 	    }
 	}
 	return systemId;
diff --git a/org/madore/damlengine/TodoAttr.java b/org/madore/damlengine/TodoAttr.java
index 2e4cfb8..f510cf9 100644
--- a/org/madore/damlengine/TodoAttr.java
+++ b/org/madore/damlengine/TodoAttr.java
@@ -13,15 +13,15 @@ public abstract class TodoAttr extends TodoItem {
 					 TodoItem caller);
     }
 
-    protected static Map<String,Factory> damlAttrFactories;
+    protected final static Map<String,Factory> damlAttrFactories;
 
     static {
 	damlAttrFactories = new HashMap<String,Factory>();
 	damlAttrFactories.put("xempty", new TodoXemptyAttr.Factory());
     }
 
-    Attr attr;
-    Element owner;
+    protected final Attr attr;
+    protected final Element owner;
 
     public TodoAttr(Attr attr, Element owner,
 		    Context ctx, TodoItem caller) {
diff --git a/org/madore/damlengine/TodoBodyElement.java b/org/madore/damlengine/TodoBodyElement.java
index be1a563..05073c8 100644
--- a/org/madore/damlengine/TodoBodyElement.java
+++ b/org/madore/damlengine/TodoBodyElement.java
@@ -4,9 +4,10 @@ import java.util.ArrayList;
 import java.util.regex.Pattern;
 import org.w3c.dom.*;
 
-public class TodoBodyElement extends TodoDefaultElement {
+public final class TodoBodyElement extends TodoDefaultElement {
 
     public static class Factory extends TodoElement.Factory {
+	@Override
 	public TodoBodyElement newItem(Element node,
 				       Context ctx,
 				       TodoItem caller) {
@@ -20,6 +21,7 @@ public class TodoBodyElement extends TodoDefaultElement {
 	super(node, ctx, caller);
     }
 
+    @Override
     public void handleNodeOnly() {
 	if ( ! ( caller instanceof TodoDamlElement ) )
 	    throw new IllegalArgumentException("body node can only be child of daml node");
diff --git a/org/madore/damlengine/TodoDamlElement.java b/org/madore/damlengine/TodoDamlElement.java
index 419e7d3..9885f36 100644
--- a/org/madore/damlengine/TodoDamlElement.java
+++ b/org/madore/damlengine/TodoDamlElement.java
@@ -4,9 +4,10 @@ import java.util.ArrayList;
 import java.util.regex.Pattern;
 import org.w3c.dom.*;
 
-public class TodoDamlElement extends TodoDefaultElement {
+public final class TodoDamlElement extends TodoDefaultElement {
 
     public static class Factory extends TodoElement.Factory {
+	@Override
 	public TodoDamlElement newItem(Element node,
 				       Context ctx,
 				       TodoItem caller) {
@@ -20,6 +21,7 @@ public class TodoDamlElement extends TodoDefaultElement {
 	super(node, ctx, caller);
     }
 
+    @Override
     public void handleNodeOnly() {
 	if ( ! ( caller instanceof DamlEngine.RootTodo ) )
 	    throw new IllegalArgumentException("daml node can only be root node");
diff --git a/org/madore/damlengine/TodoDefaultElement.java b/org/madore/damlengine/TodoDefaultElement.java
index 4360fe5..7cd9aae 100644
--- a/org/madore/damlengine/TodoDefaultElement.java
+++ b/org/madore/damlengine/TodoDefaultElement.java
@@ -6,6 +6,7 @@ import org.w3c.dom.*;
 public class TodoDefaultElement extends TodoElement {
 
     public static class Factory extends TodoElement.Factory {
+	@Override
 	public TodoDefaultElement newItem(Element node,
 					  Context ctx,
 					  TodoItem caller) {
@@ -62,6 +63,7 @@ public class TodoDefaultElement extends TodoElement {
 	this.ownerDeque.registerAtStart(toProcess);
     }
 
+    @Override
     public void handle() {
 	assert(this.ownerDeque != null);
 	handleAttributes();
diff --git a/org/madore/damlengine/TodoDeque.java b/org/madore/damlengine/TodoDeque.java
index 4cb1d38..3970938 100644
--- a/org/madore/damlengine/TodoDeque.java
+++ b/org/madore/damlengine/TodoDeque.java
@@ -5,31 +5,31 @@ import java.util.LinkedList;
 
 public final class TodoDeque {
 
-    private LinkedList<TodoItem> deque;
+    protected final LinkedList<TodoItem> deque;
 
     public TodoDeque() {
 	deque = new LinkedList<TodoItem>();
     }
 
     public void registerAtEnd(TodoItem it) {
-	it.ownerDeque = this;
+	it.setOwnerDeque(this);
 	deque.addLast(it);
     }
 
     public void registerAtEnd(Collection<? extends TodoItem> them) {
 	for ( TodoItem it : them )
-	    it.ownerDeque = this;
+	    it.setOwnerDeque(this);
 	deque.addAll(them);
     }
 
     public void registerAtStart(TodoItem it) {
-	it.ownerDeque = this;
+	it.setOwnerDeque(this);
 	deque.addFirst(it);
     }
 
     public void registerAtStart(Collection<? extends TodoItem> them) {
 	for ( TodoItem it : them )
-	    it.ownerDeque = this;
+	    it.setOwnerDeque(this);
 	deque.addAll(0, them);
     }
 
@@ -40,7 +40,7 @@ public final class TodoDeque {
     public boolean dispatchOne() {
 	TodoItem it = removeNext();
 	if ( it != null ) {
-	    assert(it.ownerDeque == this);
+	    assert(it.getOwnerDeque() == this);
 	    it.handle();
 	    return true;
 	} else
diff --git a/org/madore/damlengine/TodoElement.java b/org/madore/damlengine/TodoElement.java
index a871968..4f5d3ab 100644
--- a/org/madore/damlengine/TodoElement.java
+++ b/org/madore/damlengine/TodoElement.java
@@ -12,8 +12,8 @@ public abstract class TodoElement extends TodoItem {
 					    TodoItem caller);
     }
 
-    protected static Map<String,Factory> damlFactories;
-    protected static Factory damlDefaultFactory;
+    protected final static Map<String,Factory> damlFactories;
+    protected final static Factory damlDefaultFactory;
 
     static {
 	damlFactories = new HashMap<String,Factory>();
@@ -22,7 +22,7 @@ public abstract class TodoElement extends TodoItem {
 	damlFactories.put("body", new TodoBodyElement.Factory());
     }
 
-    Element node;
+    protected final Element node;
 
     public TodoElement(Element node,
 		       Context ctx,
diff --git a/org/madore/damlengine/TodoItem.java b/org/madore/damlengine/TodoItem.java
index 2c4bfd9..078713c 100644
--- a/org/madore/damlengine/TodoItem.java
+++ b/org/madore/damlengine/TodoItem.java
@@ -2,15 +2,25 @@ package org.madore.damlengine;
 
 public abstract class TodoItem {
 
-    public TodoDeque ownerDeque;
-    public Context ctx;
-    public TodoItem caller;
+    protected TodoDeque ownerDeque;
+    protected final Context ctx;
+    protected final TodoItem caller;
 
     public TodoItem(Context ctx, TodoItem caller) {
 	this.ctx = ctx;
 	this.caller = caller;
     }
 
+    public final TodoDeque getOwnerDeque() {
+	return this.ownerDeque;
+    }
+
+    public final void setOwnerDeque(TodoDeque ownerDeque) {
+	if ( this.ownerDeque != null )
+	    throw new IllegalStateException("item already owned by a deque");
+	this.ownerDeque = ownerDeque;
+    }
+
     public abstract void handle();
 
 }
diff --git a/org/madore/damlengine/TodoNavbar.java b/org/madore/damlengine/TodoNavbar.java
index 7c6448c..5108a33 100644
--- a/org/madore/damlengine/TodoNavbar.java
+++ b/org/madore/damlengine/TodoNavbar.java
@@ -2,7 +2,7 @@ package org.madore.damlengine;
 
 import org.w3c.dom.Element;
 
-public class TodoNavbar extends TodoElement {
+public final class TodoNavbar extends TodoElement {
 
     public TodoNavbar(Element node,
 		      Context ctx,
@@ -10,6 +10,7 @@ public class TodoNavbar extends TodoElement {
 	super(node, ctx, caller);
     }
 
+    @Override
     public void handle() {
 	Element p = ctx.doc.createElementNS(DamlEngine.XHTML_NS, "p");
 	String lang = LangHelper.getLangRec(node);
diff --git a/org/madore/damlengine/TodoStyleOrScript.java b/org/madore/damlengine/TodoStyleOrScript.java
index 57aefba..dcec137 100644
--- a/org/madore/damlengine/TodoStyleOrScript.java
+++ b/org/madore/damlengine/TodoStyleOrScript.java
@@ -2,11 +2,25 @@ package org.madore.damlengine;
 
 import org.w3c.dom.*;
 
-public class TodoStyleOrScript extends TodoItem {
+public final class TodoStyleOrScript extends TodoItem {
 
-    public enum Type { STYLE, SCRIPT }
+    public enum Type {
+	STYLE("style", "text/css", "/* ", " */\n"),
+	SCRIPT("script", "text/javascript", "// ", "\n");
+	final String eltName;
+	final String mimeType;
+	final String preCdata;
+	final String postCdata;
+	Type(String eltName, String mimeType,
+	     String preCdata, String postCdata) {
+	    this.eltName = eltName;
+	    this.mimeType = mimeType;
+	    this.preCdata = preCdata;
+	    this.postCdata = postCdata;
+	}
+    }
 
-    Type t;
+    final Type t;
 
     public TodoStyleOrScript(Type t,
 			     Context ctx,
@@ -15,27 +29,24 @@ public class TodoStyleOrScript extends TodoItem {
 	this.t = t;
     }
 
+    @Override
     public void handle() {
 	if ( ctx.headNode == null )
-	    throw new IllegalArgumentException("head node is null when doing style or script");
+	    throw new IllegalStateException("head node is null when doing style or script");
 	Element node
-	    = ctx.doc.createElementNS(DamlEngine.XHTML_NS,
-				      (t==Type.SCRIPT)?"script":"style");
-	node.setAttributeNS(null, "type",
-			    (t==Type.SCRIPT)?"text/javascript":"text/css");
+	    = ctx.doc.createElementNS(DamlEngine.XHTML_NS, t.eltName);
+	node.setAttributeNS(null, "type", t.mimeType);
 	if ( t==Type.SCRIPT )
 	    node.setAttributeNS(null, "defer", "defer");
 	ctx.headNode.appendChild(node);
 	ctx.headNode.appendChild(ctx.doc.createTextNode("\n"));
+	node.appendChild(ctx.doc.createTextNode("\n"+t.preCdata));
+	StringBuffer content
+	    = (t==Type.SCRIPT)?ctx.scriptContent:ctx.styleContent;
 	node.appendChild(ctx.doc.
-			 createTextNode((t==Type.SCRIPT)?"\n// ":"\n/* "));
-	StringBuffer content = (t==Type.SCRIPT)?ctx.scriptContent:ctx.styleContent;
-	node.appendChild(ctx.doc.
-			 createCDATASection(((t==Type.SCRIPT)?"\n":" */\n")
-					    +content
-					    +((t==Type.SCRIPT)?"// ":"/* ")));
-	node.appendChild(ctx.doc.
-			 createTextNode((t==Type.SCRIPT)?"\n":" */\n"));
+			 createCDATASection(t.postCdata+content
+					    +t.preCdata));
+	node.appendChild(ctx.doc.createTextNode(t.postCdata));
     }
 
 }
diff --git a/org/madore/damlengine/TodoXemptyAttr.java b/org/madore/damlengine/TodoXemptyAttr.java
index cc8491c..a9f7c94 100644
--- a/org/madore/damlengine/TodoXemptyAttr.java
+++ b/org/madore/damlengine/TodoXemptyAttr.java
@@ -5,6 +5,7 @@ import org.w3c.dom.*;
 public class TodoXemptyAttr extends TodoAttr {
 
     public static class Factory extends TodoAttr.Factory {
+	@Override
 	public TodoXemptyAttr newItem(Attr attr, Element owner,
 				      Context ctx,
 				      TodoItem caller) {
@@ -18,6 +19,7 @@ public class TodoXemptyAttr extends TodoAttr {
 	super(attr, owner, ctx, caller);
     }
 
+    @Override
     public void handle() {
 	this.owner.removeAttribute(this.attr.getName());
 	this.owner.appendChild(ctx.doc.createComment(" EMPTY "));
diff --git a/org/madore/damlengine/Unparser.java b/org/madore/damlengine/Unparser.java
index 2cfe447..6282117 100644
--- a/org/madore/damlengine/Unparser.java
+++ b/org/madore/damlengine/Unparser.java
@@ -6,11 +6,11 @@ import org.w3c.dom.*;
 
 public final class Unparser {
 
-    private Document doc;
+    private final Document doc;
     private Node cursor;
     private enum Dir { PUSHING, POPPING }
     private Dir dir;
-    private Writer out;
+    private final Writer out;
 
     public Unparser(Document doc, Writer out) {
 	this.doc = doc;