[jdom-interest] non-deterministic behaviour [eg]

[Jools]

Jason Hunter wrote:
> 
> Jools wrote:
> >
> > Hi all
> >
> > I'd like a little feedback as to what state we
> > shoukd leave a JDOM object, should an update operation
> > partially fail.
> >
> > Below is the code for setMixedContent. I have a
> > little concern with the exception thrown at the bottom
> > of the for(;;) loop, please take a look;
> >
> > My concern is this;
> >
> > When setMixedContent is called we take it for granted
> > that we will successfully complete the operation and
> > the Element will contain all the right data, however
> > should the operation fail the original data will have
> > been lost as we clear the list at the start of the
> > operation.
> >
> > My worry is that this is non-deterministic behaviour
> > as an Element could end up in a bit of mess should
> > the above senario take place.
> >
> > Would it be better to just ignore Objects which are
> > not JDOM objects ? Or do we need to keep a copy
> > of the original data before we commit to the new
> > data, should some of it be non-JDOM classes ?
> 
> In this specific example, since setMixedContent() does a full content
> replacement, it'd be just as efficient code-wise to keep the original
> list data and restore it in event of an illegal add.  It seems a worthy
> thing to do.

In this instance I'd have to agree.
 
> The trickier situation is the elt.getChildren().add(list) call where
> you're modifying an existing list so you'll have to do a non-deep
> clone() of the original list to keep its contents or pre-scan the list
> being added.  That adds a performance hit always even though adding
> wrongly typed elements is an edge case.  Hmm... Probably it's worth it
> to do the clone() for consistency, and besides you don't generaly call
> add(list) that often and specifically it's not used during the
> time-critical build phase.
> 
> Maybe you could even be smart and do a clone() if the existing list is
> short and do a scan-for-valid-first if the list being added is short.

Sure, nice idea. But how short is short ? I like the
scan-for-valid-first
idea, however it does add CPU overhead as you would need to iterate the
list twice, once to check it then again to add the data to the list.

However take this for example;

Exception stuffed = null;
try {
  // save current data 
  // add the data
} catch(Exception e) {
  stuffed = e;
} finally {
   if (stuffed != null) {
      // restore original data
      // throw an IllegalAddException
   }
}

We take a copy of the list, then add the data using the verification
already aforded to us by the existing code, if we get an exception
the finally clause puts back the original data, then throws and
exception.

I think I prefer the above.
 
> Note that a Node interface wouldn't help here because someone could
> still create a class Foo implements Node that makes no sense being added
> to a document.

Not sure I follow.... Other than trying to stop another 'lets use 
interfaces thread' :-)

--Jools