Multiple copies of "PreCondition" file

[GoogleCodeExporter]

By my count, there are five copies (find . -iname PreCondition.java).

We should only need one. Perhaps just the one in the genxdm-api package.

I've marked this as a defect, because of the risk that these implementations 
could diverge.

Original issue reported on code.google.com by eric%tib...@gtempaccount.com on 18 Jan 2012 at 4:06

[GoogleCodeExporter]

I briefly considered whether we should use the Java "assert" keyword. The 
primary problem with that approach is that Java defaults "assert" to "off". 
However, given that we expect XML processing code to be performance sensitive, 
the current approach of always having them on means that we cannot boost our 
performance by removing them.

Thoughts?

Original comment by eric%tib...@gtempaccount.com on 18 Jan 2012 at 4:12

[GoogleCodeExporter]

There are currently four copies: apart from the one in the core api, there are 
three in the regex implementation in the schema parser, and one in the 
validation implementation. Removing them should be easily enough done.

If we want to use assert, I'd suggest that we should change PreCondition to do 
so. But I don't know that we *do* want to do that. Yes, we can improve 
performance that way, but we lose correctness. Unfortunately (in my experience, 
at least), the folks who want things to go fast aren't the ones who are good at 
insuring that their inputs are clean.

Original comment by aale...@gmail.com on 15 Jun 2012 at 3:27

[GoogleCodeExporter]

Switching to "assert" would actually require changes elsewhere, as some parts 
of bridge implementations are relying on the PreCondition "throw" behavior to 
conform to contract.

Having discovered that particular detail since I filed the bug, I recommend 
that we explicitly throw exceptions.

Original comment by eric%tib...@gtempaccount.com on 15 Jun 2012 at 9:34

[GoogleCodeExporter]

Okay. So, we still have to kill the four extras.

As to explicitly throwing: we do so, but what we throw runs to extremes.

GenXDMException extends RuntimeException. It's easy to hide it in an 
implementation, just by not mentioning it in a 'throws' clause. A couple of the 
methods in PreCondition return exceptions (which can be thrown), which extend 
IllegalArgumentException, also a Runtime.  See issue 72 for more discussion 
about that.

Most methods in PreCondition, though throw (not *return*) AssertionError, which 
(according to the JDK docco for Error) "a reasonable application should not try 
to catch."  No checked exceptions in this mix, just straight from runtime 
exceptions to errors.  It's an almost xml-like approach to problems: if there 
are any, just stop, don't attempt recovery.

Original comment by aale...@gmail.com on 15 Jun 2012 at 10:42

[GoogleCodeExporter]

Still need to do this, and it's easy enough. Keep it in 1.0. It's just a matter 
of changing the imports and then deleting redundant stuff.

Original comment by aale...@gmail.com on 26 Jul 2012 at 8:01

• Added labels: Milestone-Release1.0

[GoogleCodeExporter]

This issue was closed by revision r347.

Original comment by aale...@gmail.com on 30 Jul 2012 at 8:14

• Changed state: Fixed