[jdom-interest] More changes

[Jason Hunter]

> > On a somewhat related note, I'm really starting to like the idea of
> > getChildContent(String childName) and getChildContent(String childName,
> > String fallback).  These methods would greatly simplify my coding when
> > reading config files.
> 
> Ughhh... why? What's wrong with
> 
> element.getChild(childName).getContent(); ?

What's wrong is that getChild() can now return null, and so every time
you do that call chain you're risking an NPE.  So now it's become

String value = "default";
Element child = element.getChild(childName);
if (child != null) {
  value = child.getContent();
}

There's the Ughhh.  :-)  What I really want is

element.getChildContent(childName, "default");

This is particularly useful in config file parsing.

        <init-param>
            <param-name>
                initial
            </param-name>
            <param-value>
                1000
            </param-value>
            <description>
                The initial value for the counter  <!-- optional -->
            </description>
        </init-param>

OK, I'm going to do some experimenting here in real time seeing how
various approaches can solve this problem.  There are other problems to
solve, of course, but this is a good data point.

The current approach to reading a config file like this (with a style
that avoids throwing/catching any NPE because I believe that is
something you want to avoid) would be this:

Element ip = servlet.getChild("init-param");
String name = value = desc = null;
if (ip != null) {
  Element pn = ip.getChild("param-name");
  Element pv = ip.getChild("param-value");
  Element ds = ip.getChild("description");
  if (pn != null) {
    name = pn.getContent();
  }
  if (pv != null) {
    value = pv.getContent();
  }
  if (desc != null) {
    desc = pv.getContent();
  }
  else {
    desc = "No description";
  }
  if (name != null && value != null) {
    // Handle valid init parameter
  }
  else {
    // Handle invalid init parameter
  }
}
else {
  // Handle missing init parameter
}

Not the best code.  :-)  I don't think I missed any easy optimizations
either.  Now let's try it allowing the (bad) idea of an NPE try/catch
seeing if that simplifies things:

Element ip = servlet.getChild("init-param");
String name = value = desc = null;
if (ip != null) {
  try {
    name = ip.getChild("param-name").getContent();
    value = ip.getChild("param-value").getContent();
  }
  catch (NullPointerException e) {
    // Handle missing p-n or p-v although you can't tell which!
    // Plus it's hard to properly return from this block
  }
  try {
    desc = ip.getChild("description").getContent();
  }
  catch (NullPointerException e) {
    desc = "No description";
  }
  // Handle valid init parameter here
}
else {
  // Handle missing init parameter
}

Not any better imho.  Now let's try my proposal:

Element ip = servlet.getChild("init-param");
String name = value = desc = null;
if (ip != null) {
  name = ip.getChildContent("param-name"); // may return null
  value = ip.getChildContent("param-value");
  desc = ip.getChildContent("description", "No description");
}
if (name != null && value != null) {
  // Handle good input here
}
else {
  // Handle missing elements
}

I like that code a whole lot more.  Just for fun, I want to see what
happens if we hadn't removed the NoSuchXXXExceptions.

String name = value = desc = null;
try {
  Element ip = servlet.getChild("init-param");
  name = ip.getChild("param-name").getContent();
  value = ip.getChild("param-value").getContent();
  desc = ip.get"No Description";  // default
  try {
    String desc = ip.getChild("description").getContent();
  } catch (NoSuchChildException e) {}
  // Handle good input here
}
catch (NoSuchChildException e) {
  // Handle missing elements
}

Not very pretty.  Now since I'm in a groove, let's try writing the code
with my proposal *and* the NoSuchXXXExceptions added back in.

String name = value = desc = null;
try {
  Element ip = servlet.getChild("init-param");  // may throw NSCE
  name = ip.getChildContent("param-name");  // now may throw NSCE
  value = ip.getChildContent("param-value");
  desc = ip.getChildContent("description", "No description");
  // Handle good input here
}
catch (NoSuchChildException e) {
  // Handle missing elements (message says which one)
}

In my opinion, "my proposal and returning null" and "my proposal and
throwing exceptions" both make good clean code for this use case. 
Anyone disagree with my little test?  Anyone want to do a test like this
for another simple but common use case?

-jh-