From f2745aadc7d0eb02002f667cd72d8536e4f1daf1 Mon Sep 17 00:00:00 2001 From: "David A. Madore" Date: Thu, 22 Apr 2010 02:16:29 +0200 Subject: 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. --- org/madore/damlengine/DamlEngine.java | 4 ++- org/madore/damlengine/LangHelper.java | 4 +-- org/madore/damlengine/Resolver.java | 12 +++++--- org/madore/damlengine/TodoAttr.java | 6 ++-- org/madore/damlengine/TodoBodyElement.java | 4 ++- org/madore/damlengine/TodoDamlElement.java | 4 ++- org/madore/damlengine/TodoDefaultElement.java | 2 ++ org/madore/damlengine/TodoDeque.java | 12 ++++---- org/madore/damlengine/TodoElement.java | 6 ++-- org/madore/damlengine/TodoItem.java | 16 ++++++++-- org/madore/damlengine/TodoNavbar.java | 3 +- org/madore/damlengine/TodoStyleOrScript.java | 43 +++++++++++++++++---------- org/madore/damlengine/TodoXemptyAttr.java | 2 ++ org/madore/damlengine/Unparser.java | 4 +-- 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 damlAttrFactories; + protected final static Map damlAttrFactories; static { damlAttrFactories = new HashMap(); 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 deque; + protected final LinkedList deque; public TodoDeque() { deque = new LinkedList(); } public void registerAtEnd(TodoItem it) { - it.ownerDeque = this; + it.setOwnerDeque(this); deque.addLast(it); } public void registerAtEnd(Collection 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 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 damlFactories; - protected static Factory damlDefaultFactory; + protected final static Map damlFactories; + protected final static Factory damlDefaultFactory; static { damlFactories = new HashMap(); @@ -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; -- cgit v1.2.3