[jdom-interest] More changes

[Brett McLaughlin]

Jason Hunter wrote:
> 
> > > 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");

OK - as nicely as possible ;-), this is the consequence of the null vs.
Exception discussion - this sort of thing is the logical end-result. I
agree that getChildContent() is fraught with confusion and
double-meaning. Is the child returned the first child? SO if you want
the second child you can't do this shortcut? Is the default returned
when the value doesn't exist? when the child doesn't exist? When there's
an error? Is the default an Element? or a value? Is that really
accurate?

I think if this is really a problem, than what you are saying is that
you want an Exception to be thrown. I really don't see an in between
without it being a hack, which is what this looks like.

-Brett

> 
> 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-

-- 
Brett McLaughlin, Enhydra Strategist
Lutris Technologies, Inc. 
1200 Pacific Avenue, Suite 300 
Santa Cruz, CA 95060 USA 
http://www.lutris.com
http://www.enhydra.org