From f2745aadc7d0eb02002f667cd72d8536e4f1daf1 Mon Sep 17 00:00:00 2001
From: "David A. Madore" <david+git@madore.org>
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(-)
(limited to 'org')
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;
--
cgit v1.2.3