crosswire / jsword

JSword
http://www.crosswire.org/jsword/
48 stars 62 forks source link

Book.getGlobalKeyList().iterator() misses last verse(s) of testament #91

Closed schierlm closed 5 years ago

schierlm commented 9 years ago

Hello,

when using Book.getGlobalKeyList().iterator to iterate over all verses of the KJV bible, it misses Malachi 4:6 and Revelation 22:20 and Revelation 22:21. So, the last verse of OT and the last two verses of NT are missing. When explicitly creating Verse objects and calling Book.contains(), it tells me that these verses are there, though.

Happens for other bibles/versifications as well.

Probably some index off-by-one in the Versifications modules somewhere.

Tested in latest commit (748341fc90e253d9fcd866f260cc9c0e4af65f00)

dmsmith commented 9 years ago

Many thanks for reporting this. And for your contributions at github.

Hope to be able to look at this soon.

In Him, DM

On Feb 7, 2015, at 3:10 PM, Michael Schierl notifications@github.com wrote:

Hello,

when using Book.getGlobalKeyList().iterator to iterate over all verses of the KJV bible, it misses Malachi 4:6 and Revelation 22:20 and Revelation 22:21. So, the last verse of OT and the last two verses of NT are missing. When explicitly creating Verse objects and calling Book.contains(), it tells me that these verses are there, though.

Happens for other bibles/versifications as well.

Probably some index off-by-one in the Versifications modules somewhere.

Tested in latest commit (748341f https://github.com/crosswire/jsword/commit/748341fc90e253d9fcd866f260cc9c0e4af65f00)

— Reply to this email directly or view it on GitHub https://github.com/crosswire/jsword/issues/91.

schierlm commented 9 years ago

Did you find time to have a look at this? It seems to me (empirically) that it works if you add 1 to the expressions in https://github.com/crosswire/jsword/blob/master/src/main/java/org/crosswire/jsword/book/sword/ZVerseBackend.java#L231 and https://github.com/crosswire/jsword/blob/master/src/main/java/org/crosswire/jsword/versification/Versification.java#L879, but not sure if it is all places to fix and/or other places need adjusting when using different API calls...

schierlm commented 6 years ago

Any updates on this (or any workarounds)? This is still broken in the latest version (1983dc1bd39d735a28accadbe282f4a043b225a9)

EDIT This workaround seems to reliably find all verses of a book (but is a bit slow):

List<Verse> allVerses = new ArrayList<>();
Verse nextCandidate = null;
for (Iterator<?> iter = book.getGlobalKeyList().iterator(); iter.hasNext();) {
    Verse verse = (Verse) iter.next();
    while (nextCandidate != null && !nextCandidate.equals(verse)) {
        if (book.contains(nextCandidate)) {
            System.out.println("WARNING: Verse (after) skipped by iterator: "+nextCandidate);
            allVerses.add(nextCandidate);
        }
        nextCandidate = nextCandidate.getVersification().add(nextCandidate, 1);
    }
    Verse prevCandidate = verse.getVersification().subtract(verse, 1);
    List<Verse> versesSkippedBefore = new ArrayList<>();
    while(prevCandidate != null && !allVerses.contains(prevCandidate) && !versesSkippedBefore.contains(prevCandidate)) {
        versesSkippedBefore.add(0, prevCandidate);
        prevCandidate = prevCandidate.getVersification().subtract(prevCandidate, 1);
    }
    for (Verse v : versesSkippedBefore) {
        if (book.contains(v)) {
            System.out.println("WARNING: Verse (before) skipped by iterator: "+v);
            allVerses.add(v);
        }
    }
    allVerses.add(verse);
    nextCandidate = verse.getVersification().add(verse, 1);
}
while (nextCandidate != null) {
    if (book.contains(nextCandidate)) {
        System.out.println("WARNING: Verse (at end) skipped by iterator: " + nextCandidate);
        allVerses.add(nextCandidate);
    }
    Verse nextNextCandidate = nextCandidate.getVersification().add(nextCandidate, 1);
    if (nextCandidate.equals(nextNextCandidate))
        break;
    nextCandidate = nextNextCandidate;
}
dmsmith commented 5 years ago

verified. Sorry it took so long.

dmsmith commented 5 years ago

Fixed. The problem was within the raw and compressed backend for Bibles in the getGlobalKeyList. The index 0 was not being counted as a verse, yet it is in the index file. Further, entrySize maxIndex was wrong. It should have been entrySize maxCount, because there is one entry per verse, including the skipped entry at the beginning of the index.

schierlm commented 5 years ago

Fixed in which commit? fce9d871d687f2f961136776a024b4b5eee1f37c does not seem to fix it. I'm still seeing this when trying to iterate over KJV module with jsword revision dbd5f3d30f28acd353f6437ce02488d169174c35

schierlm commented 5 years ago

Please ignore my previous message. I somehow managed to have two different versions of jsword on my ClassPath :-(