<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">See embedded comments.<br><div><div>On Sep 5, 2012, at 11:45 AM, Chris Burrell <<a href="mailto:chris@burrell.me.uk">chris@burrell.me.uk</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">On the below, I was tempted, to fix this by using the distance() on the versification. However, that does not seem to work. Not sure why. Here was my code for VerseRange constructor:<div><br></div><div><div> int distance = v11n.distance(start, end);</div></div></blockquote><div><br></div>compareTo is not v11n safe. It should be marked as deprecated. Or a verse needs to have a v11n. I've been trying to avoid that and have a verse be a dumb holder of a Book, chapter number and a verse number.</div><div><br></div><div>Using v11n.distance(start, end) is the replacement for compareTo.</div><div><br></div><div>This code looks correct.</div><div><br><blockquote type="cite"><div>
<div> if(distance < 0) {</div><div> this.start = end;</div><div> this.end = start;</div><div> this.verseCount = calcVerseCount();</div><div> } else if (distance == 0) {</div>
<div> this.start = start;</div><div> this.end = start;</div><div> this.verseCount = 1;</div><div> } else {</div><div> this.start = end;</div><div> this.end = start;</div>
<div> this.verseCount = calcVerseCount();</div><div> }</div><div><br></div><div>Loads of tests then start failing. </div></div></blockquote><div><br></div>The test might be wrong. Or only right for the KJV versification.</div><div><br></div><div><br><blockquote type="cite"><div><div>Clearly, that's not quite replacing the constructor, like for like.</div></div></blockquote><div><br></div>Looking at it, it appears that the problem is in the bowels of v11n.getOrdinal(verse).</div><div><br></div><div>Here is the code:</div><div><div style="margin: 0px; font-size: 11px; font-family: Monaco; "> <span style="color: #931a68">public</span> <span style="color: #931a68">int</span> getOrdinal(Verse verse) {</div><div style="margin: 0px; font-size: 11px; font-family: Monaco; "> <span style="color: #931a68">return</span> <span style="color: #0326cc">chapterStarts</span>[<span style="color: #0326cc">bookList</span>.getOrdinal(verse.getBook())][verse.getChapter()] + verse.getVerse();</div><div style="margin: 0px; font-size: 11px; font-family: Monaco; "> }</div><div><br></div><div>Obviously, the index for the book and the chapter have to be in bounds. One of them isn't. This method should not throw an AIOBE.</div><div><br></div><div>Looking at bookList.getOrdinal, it will return -1 if the book is not in the versification.</div><div>bookList.contains(book) will determine whether the book is in the versification or not.</div><div><br></div><div>If the book is not in the versification, there's not much that can be done. We cannot figure out whether to pick the first or last book in a testament. In fact, we cannot know which testament in which the book belongs.</div><div><br></div><div>verse.getChapter() is a bit simpler. It merely returns the chapter number contained in the verse. It should always be >=0. But it might be too big.</div><div><br></div><div>We have code to patch up such an out of range reference. See v11n.patch(Book, chapter, verse). (Probably the argument list should just be Verse) Basic idea: When the number was bigger than the book, we kept going into the next book. Likewise for the verse number.</div><div><br></div><div>We thought that the results were very surprising to an end user. The user makes a mistake or asks for something that isn't valid and is given something else without warning. So it is currently not used.</div><div><br></div><div>Note that the way that the verse is added in. It doesn't check to see whether the reference is in bounds of the chapter, let alone the v11n. So in a way, it "patches."</div><div><br></div><div>In v11n, there is also a validate method that will throw an NoSuchVerseException if the parts don't make sense for the v11n. </div><div><br></div><div>Both validate and patch don't properly handle a book not in the versification. (i.e. bug in there.)</div><div><br></div><div>So, getOrdinal(verse) needs to be changed. Perhaps, wrap w/ try catching AIOBE and then at least logging the error and returning something marginally reasonable.</div><div><br></div><div>In the past we used to throw a NoSuchVerseException via validate when constructing a verse. Now that a verse does not have a v11n context that was removed.</div><div><br></div><div>In Him,</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>DM</div><div><br></div><div><br></div><blockquote type="cite"><div><div><br></div>
<div>Chris</div><div><br></div><br><div class="gmail_quote">On 5 September 2012 16:07, Chris Burrell <span dir="ltr"><<a href="mailto:chris@burrell.me.uk" target="_blank">chris@burrell.me.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; ">
Hi<div><br></div><div>While I'm trying to retrieve a passage it seems to want to compare some keys. I'm looking at the DRC module, with Tob 1, but the problem occurs elsewhere.</div><div><br></div><div>Here's the stack trace I get. I'm at a lost to why we need to compare verse numbers here, but perhaps someone can enlighten me. Happy to fix it if I can understand what it's trying to do and a way forward.</div>
<div><br></div><div>Also, I've made a small fix to the Versification elsewhere in the code (pull request in Git).</div><div><br></div><div><div>Caused by: java.lang.ArrayIndexOutOfBoundsException: -1</div><div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.versification.Versification.getOrdinal(Versification.java:503)</div>
<div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.passage.Verse.getOrdinal(Verse.java:440)</div><div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.passage.Verse.compareTo(Verse.java:261)</div>
<div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.passage.VerseRange.<init>(VerseRange.java:136)</div><div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.passage.VerseRangeFactory.fromText(VerseRangeFactory.java:124)</div>
<div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.passage.VerseRangeFactory.fromString(VerseRangeFactory.java:96)</div><div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.passage.VerseRangeFactory.fromString(VerseRangeFactory.java:61)</div>
<div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.passage.AbstractPassage.addVerses(AbstractPassage.java:879)</div><div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.passage.BitwisePassage.<init>(BitwisePassage.java:88)</div>
<div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.passage.RocketPassage.<init>(RocketPassage.java:70)</div><div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.passage.PassageType$1.createPassage(PassageType.java:44)</div>
<div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.passage.PassageKeyFactory.getKey(PassageKeyFactory.java:83)</div><div><span style="white-space:pre-wrap"> </span>at org.crosswire.jsword.book.basic.AbstractPassageBook.getKey(AbstractPassageBook.java:217)</div>
<div><span style="white-space:pre-wrap"> </span>at com.tyndalehouse.step.core.service.jsword.impl.JSwordPassageServiceImpl.getBookData(JSwordPassageServiceImpl.java:447)</div><div><span style="white-space:pre-wrap"> </span>at com.tyndalehouse.step.core.service.jsword.impl.JSwordPassageServiceImpl.getOsisText(JSwordPassageServiceImpl.java:433)</div>
<div><span style="white-space:pre-wrap"> </span>at com.tyndalehouse.step.core.service.impl.BibleInformationServiceImpl.getPassageText(BibleInformationServiceImpl.java:128)</div><div><span style="white-space:pre-wrap"> </span>at com.tyndalehouse.step.rest.controllers.BibleController.getBibleText(BibleController.java:157)</div>
<div><span style="white-space:pre-wrap"> </span>at com.tyndalehouse.step.rest.controllers.BibleController.getBibleText(BibleController.java:138)</div><div><span style="white-space:pre-wrap"> </span>... 30 more</div>
</div><div><br></div><div><br></div><div>Crhis</div><div><br></div>
</blockquote></div><br></div>
_______________________________________________<br>jsword-devel mailing list<br><a href="mailto:jsword-devel@crosswire.org">jsword-devel@crosswire.org</a><br>http://www.crosswire.org/mailman/listinfo/jsword-devel<br></blockquote></div><br></body></html>