From 07b9d049330655b496eeb45c180baae2e1d3413a Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Sat, 22 Nov 2014 00:21:38 -0500 Subject: [PATCH] Add some slight refactoring Unfortunately it's too hard to add the tests I wanted to --- .../format/osisparser/OsisParserTest.java | 27 ---------------- .../activity/viewer/BibleViewerModules.java | 3 +- .../service/format/osisparser/OsisParser.kt | 15 ++++++--- .../service/lookup/VerseLookup.kt | 31 ++++++++++--------- 4 files changed, 27 insertions(+), 49 deletions(-) diff --git a/app-test/src/test/java/org/bspeice/minimalbible/test/format/osisparser/OsisParserTest.java b/app-test/src/test/java/org/bspeice/minimalbible/test/format/osisparser/OsisParserTest.java index 670e1af..80d1822 100644 --- a/app-test/src/test/java/org/bspeice/minimalbible/test/format/osisparser/OsisParserTest.java +++ b/app-test/src/test/java/org/bspeice/minimalbible/test/format/osisparser/OsisParserTest.java @@ -4,13 +4,10 @@ import android.annotation.SuppressLint; import org.bspeice.minimalbible.service.format.osisparser.OsisParser; import org.crosswire.jsword.book.OSISUtil; -import org.crosswire.jsword.passage.Verse; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.xml.sax.Attributes; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; @@ -58,28 +55,4 @@ public class OsisParserTest { parser.endElement("", "", ""); assertTrue(parser.getDoWrite().isEmpty()); } - - // During initial development, I accidentally set up the verseContent - // as a value computed every time - so you'd get a new "content" every time - // you tried to update it. Thus, if you updated the content only once, you're fine. - // Try and update multiple times, and things would start going crazy. - // TODO: Why is this ignored? - @SuppressLint("NewApi") - @SuppressWarnings("unused") - @Ignore - public void ignoreTestVerseContentConsistent() { - String string1 = "1"; - String string2 = "2"; - - // Yes, I need intimate knowledge of the state machine to run this test - // Also, Verse is final, so I can't mock that, which is why this test is ignored. - Verse mockVerse = mock(Verse.class); - parser.setVerse(mockVerse); - parser.getDoWrite().push(true); - parser.characters(string1.toCharArray(), 0, string1.length()); - assertEquals(parser.getVerseContent().getContent(), string1); - - parser.characters(string2.toCharArray(), 0, string2.length()); - assertEquals(parser.getVerseContent().getContent(), string1 + string2); - } } diff --git a/app/src/main/java/org/bspeice/minimalbible/activity/viewer/BibleViewerModules.java b/app/src/main/java/org/bspeice/minimalbible/activity/viewer/BibleViewerModules.java index b28da15..bbf7204 100644 --- a/app/src/main/java/org/bspeice/minimalbible/activity/viewer/BibleViewerModules.java +++ b/app/src/main/java/org/bspeice/minimalbible/activity/viewer/BibleViewerModules.java @@ -2,7 +2,6 @@ package org.bspeice.minimalbible.activity.viewer; import android.util.Log; -import org.bspeice.minimalbible.service.lookup.DefaultVerseLookup; import org.bspeice.minimalbible.service.lookup.VerseLookup; import org.bspeice.minimalbible.service.manager.BookManager; import org.crosswire.jsword.book.Book; @@ -100,6 +99,6 @@ public class BibleViewerModules { @Provides @Singleton VerseLookup verseLookup(@Named("MainBook") Book b) { - return new DefaultVerseLookup(b); + return new VerseLookup(b); } } \ No newline at end of file diff --git a/app/src/main/kotlin/org/bspeice/minimalbible/service/format/osisparser/OsisParser.kt b/app/src/main/kotlin/org/bspeice/minimalbible/service/format/osisparser/OsisParser.kt index 28872e4..76738ee 100644 --- a/app/src/main/kotlin/org/bspeice/minimalbible/service/format/osisparser/OsisParser.kt +++ b/app/src/main/kotlin/org/bspeice/minimalbible/service/format/osisparser/OsisParser.kt @@ -6,6 +6,8 @@ import java.util.ArrayDeque import org.xml.sax.Attributes import org.crosswire.jsword.book.OSISUtil import org.bspeice.minimalbible.SafeValDelegate +import org.crosswire.jsword.book.BookData +import org.crosswire.jsword.book.Book /** * Created by bspeice on 9/10/14. @@ -15,13 +17,17 @@ class OsisParser() : DefaultHandler() { // Don't pass a verse as part of the constructor, but still guarantee // that it will exist - public var verse: Verse by SafeValDelegate() - var verseContent: VerseContent? = null + var verseContent: VerseContent by SafeValDelegate() // TODO: Implement a stack to keep min API 8 val doWrite = ArrayDeque() - fun getJson() = verseContent!!.json + fun getJson(b: Book, v: Verse): String { + // I don't always set up my constructors to have faces, but when I do... + verseContent = VerseContent(v = v) + BookData(b, v).getSAXEventProvider() provideSAXEvents this + return verseContent.json + } override fun startElement(uri: String, localName: String, qName: String, attributes: Attributes) { @@ -37,7 +43,6 @@ class OsisParser() : DefaultHandler() { override fun characters(ch: CharArray, start: Int, length: Int) { if (doWrite.peek()) - verseContent = verseContent?.appendContent(String(ch)) ?: - VerseContent(verse, content = String(ch)) + verseContent = verseContent appendContent String(ch) } } \ No newline at end of file diff --git a/app/src/main/kotlin/org/bspeice/minimalbible/service/lookup/VerseLookup.kt b/app/src/main/kotlin/org/bspeice/minimalbible/service/lookup/VerseLookup.kt index b34493f..faf1145 100644 --- a/app/src/main/kotlin/org/bspeice/minimalbible/service/lookup/VerseLookup.kt +++ b/app/src/main/kotlin/org/bspeice/minimalbible/service/lookup/VerseLookup.kt @@ -6,14 +6,16 @@ import rx.functions.Action1 import org.crosswire.jsword.passage.Verse import rx.subjects.PublishSubject import rx.schedulers.Schedulers -import org.crosswire.jsword.book.BookData import org.bspeice.minimalbible.service.format.osisparser.OsisParser +import org.crosswire.jsword.book.getVersification /** - * Created by bspeice on 11/12/14. + * Do the low-level work of getting a verse's content + * This class is currently impossible to test because I can't mock Verse objects */ -open class VerseLookup(val b: Book, - val cache: LruCache = LruCache(1000000)) : Action1 { +open class VerseLookup(val b: Book) : Action1 { + + val cache = VerseCache() /** * The listener servers to let other objects notify us we should pre-cache verses */ @@ -27,7 +29,7 @@ open class VerseLookup(val b: Book, fun getVerseId(v: Verse) = v.getOrdinal() fun getJson(v: Verse): String = - if (contains(v)) + if (cache contains v) cache[getVerseId(v)] else { val content = doLookup(v) @@ -43,14 +45,9 @@ open class VerseLookup(val b: Book, * @param v The verse to look up * @return The JSON content of this verse */ - fun doLookup(v: Verse): String { - val data = BookData(b, v) - val provider = data.getSAXEventProvider() - val handler = OsisParser() - handler.verse = v - provider provideSAXEvents handler - return handler.getJson() - } + fun doLookup(v: Verse): String = OsisParser().getJson(b, v) + fun doLookup(ordinal: Int): String = OsisParser() + .getJson(b, Verse(b.getVersification(), ordinal)) /** * Not necessary, but helpful if you let us know ahead of time we should pre-cache a verse. @@ -68,7 +65,7 @@ open class VerseLookup(val b: Book, * @param v The verse to check * @return Whether we can retrieve the verse from our cache */ - fun contains(v: Verse) = cache[v.getOrdinal()] != null + open fun contains(v: Verse) = cache[v.getOrdinal()] != null // IO Thread operations begin here @@ -81,4 +78,8 @@ open class VerseLookup(val b: Book, } } -class DefaultVerseLookup(b: Book) : VerseLookup(b) {} +open class VerseCache : LruCache(1000000) { + + fun getId(v: Verse) = v.getOrdinal() + fun contains(v: Verse) = (this get getId(v)) != null +} \ No newline at end of file