Collections.sort(List<RocketPassage>) is reverse order - Rev first and Gen last

Lists of RocketPassages now appear to sort in inverse biblical order whereas they used to sort in Biblical order when using Collections.sort(rocketPassageList).

And Bible bookmarks seem to be deserialized to RocketPassages and when sorted using the latest JSword bookmarks in Rev appear at the top of the list and Genesis at the bottom, but with pre-av11n code bookmarks in Genesis would appear at the top.

I have dug in a little.
This is the old code in AbstractPassage.compareTo:
Verse thatfirst = thatref.getVerseAt(0);
Verse thisfirst = getVerseAt(0);

return thisfirst.compareTo(thatfirst);
Verse.compareTo:
int thatStart = that.getOrdinal();
int thisStart = this.getOrdinal();
if (thatStart > thisStart)

{ return -1; }

if (thatStart < thisStart)

{ return 1; }

And here is the new code:
Verse thatfirst = thatref.getVerseAt(0);
Verse thisfirst = getVerseAt(0);
return getVersification().distance(thisfirst, thatfirst);
Versification:
public int distance(Verse start, Verse end) {
return end.getOrdinal() - start.getOrdinal();

To see the problem imagine in the old code:
if (thatVerseinRev > thisVerseInGen) // obviously true
return -1 // so in old code the compareTo result is -ve
But in the new code:
distance(thisVerseInGen, thatVerseInRev)
thatVerseInRev.ordinal - thisVerseInGen.ordinal // result is +ve

Is it okay simply to reverse the order of that and this in AbstractPassage and make it:
return thatfirst.compareTo(thisfirst);
Will this problem be seen in any other places?

I created a couple of junits in VersificationParentTst and they seem to point to the problem only affecting lists of RocketPassage which initially surprised me until I noticed that Verse has it's own compareTo method.

// This test works correctly
public void testListSort() {
if (v11n.containsBook(BibleBook.GEN) && v11n.containsBook(BibleBook.REV))

{ List<Key> keyList = new ArrayList<Key>(); Verse gen11 = new Verse(v11n, BibleBook.GEN, 1, 1); Verse rev11 = new Verse(v11n, BibleBook.REV, 1, 1); keyList.add(gen11); keyList.add(rev11); Collections.sort(keyList); assertEquals(gen11, keyList.get(0)); assertEquals(rev11, keyList.get(1)); }

}
// This test fails because Rev is first and Gen last
public void testListSort2() {
try {
if (v11n.containsBook(BibleBook.GEN) && v11n.containsBook(BibleBook.REV))

{ List<Key> keyList = new ArrayList<Key>(); // gen11 and rev11 created this way are actually RocketPassage objects, not Verse objects Key gen11 = PassageKeyFactory.instance().getKey(v11n, "Gen.1.1"); Key rev11 = PassageKeyFactory.instance().getKey(v11n, "Rev.1.1"); keyList.add(gen11); keyList.add(rev11); Collections.sort(keyList); assertEquals(gen11, keyList.get(0)); assertEquals(rev11, keyList.get(1)); }

} catch (Exception e)

{ fail("Exception in testListSort2"); }

}

Regards
Martin