Add some slight refactoring

Unfortunately it's too hard to add the tests I wanted to
This commit is contained in:
Bradlee Speice 2014-11-22 00:21:38 -05:00
parent acbdc10116
commit 07b9d04933
4 changed files with 27 additions and 49 deletions

View File

@ -4,13 +4,10 @@ import android.annotation.SuppressLint;
import org.bspeice.minimalbible.service.format.osisparser.OsisParser; import org.bspeice.minimalbible.service.format.osisparser.OsisParser;
import org.crosswire.jsword.book.OSISUtil; import org.crosswire.jsword.book.OSISUtil;
import org.crosswire.jsword.passage.Verse;
import org.junit.Before; import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test; import org.junit.Test;
import org.xml.sax.Attributes; import org.xml.sax.Attributes;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
@ -58,28 +55,4 @@ public class OsisParserTest {
parser.endElement("", "", ""); parser.endElement("", "", "");
assertTrue(parser.getDoWrite().isEmpty()); 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);
}
} }

View File

@ -2,7 +2,6 @@ package org.bspeice.minimalbible.activity.viewer;
import android.util.Log; import android.util.Log;
import org.bspeice.minimalbible.service.lookup.DefaultVerseLookup;
import org.bspeice.minimalbible.service.lookup.VerseLookup; import org.bspeice.minimalbible.service.lookup.VerseLookup;
import org.bspeice.minimalbible.service.manager.BookManager; import org.bspeice.minimalbible.service.manager.BookManager;
import org.crosswire.jsword.book.Book; import org.crosswire.jsword.book.Book;
@ -100,6 +99,6 @@ public class BibleViewerModules {
@Provides @Provides
@Singleton @Singleton
VerseLookup verseLookup(@Named("MainBook") Book b) { VerseLookup verseLookup(@Named("MainBook") Book b) {
return new DefaultVerseLookup(b); return new VerseLookup(b);
} }
} }

View File

@ -6,6 +6,8 @@ import java.util.ArrayDeque
import org.xml.sax.Attributes import org.xml.sax.Attributes
import org.crosswire.jsword.book.OSISUtil import org.crosswire.jsword.book.OSISUtil
import org.bspeice.minimalbible.SafeValDelegate import org.bspeice.minimalbible.SafeValDelegate
import org.crosswire.jsword.book.BookData
import org.crosswire.jsword.book.Book
/** /**
* Created by bspeice on 9/10/14. * 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 // Don't pass a verse as part of the constructor, but still guarantee
// that it will exist // that it will exist
public var verse: Verse by SafeValDelegate() var verseContent: VerseContent by SafeValDelegate()
var verseContent: VerseContent? = null
// TODO: Implement a stack to keep min API 8 // TODO: Implement a stack to keep min API 8
val doWrite = ArrayDeque<Boolean>() val doWrite = ArrayDeque<Boolean>()
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, override fun startElement(uri: String, localName: String,
qName: String, attributes: Attributes) { qName: String, attributes: Attributes) {
@ -37,7 +43,6 @@ class OsisParser() : DefaultHandler() {
override fun characters(ch: CharArray, start: Int, length: Int) { override fun characters(ch: CharArray, start: Int, length: Int) {
if (doWrite.peek()) if (doWrite.peek())
verseContent = verseContent?.appendContent(String(ch)) ?: verseContent = verseContent appendContent String(ch)
VerseContent(verse, content = String(ch))
} }
} }

View File

@ -6,14 +6,16 @@ import rx.functions.Action1
import org.crosswire.jsword.passage.Verse import org.crosswire.jsword.passage.Verse
import rx.subjects.PublishSubject import rx.subjects.PublishSubject
import rx.schedulers.Schedulers import rx.schedulers.Schedulers
import org.crosswire.jsword.book.BookData
import org.bspeice.minimalbible.service.format.osisparser.OsisParser 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, open class VerseLookup(val b: Book) : Action1<Verse> {
val cache: LruCache<Int, String> = LruCache(1000000)) : Action1<Verse> {
val cache = VerseCache()
/** /**
* The listener servers to let other objects notify us we should pre-cache verses * 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 getVerseId(v: Verse) = v.getOrdinal()
fun getJson(v: Verse): String = fun getJson(v: Verse): String =
if (contains(v)) if (cache contains v)
cache[getVerseId(v)] cache[getVerseId(v)]
else { else {
val content = doLookup(v) val content = doLookup(v)
@ -43,14 +45,9 @@ open class VerseLookup(val b: Book,
* @param v The verse to look up * @param v The verse to look up
* @return The JSON content of this verse * @return The JSON content of this verse
*/ */
fun doLookup(v: Verse): String { fun doLookup(v: Verse): String = OsisParser().getJson(b, v)
val data = BookData(b, v) fun doLookup(ordinal: Int): String = OsisParser()
val provider = data.getSAXEventProvider() .getJson(b, Verse(b.getVersification(), ordinal))
val handler = OsisParser()
handler.verse = v
provider provideSAXEvents handler
return handler.getJson()
}
/** /**
* Not necessary, but helpful if you let us know ahead of time we should pre-cache a verse. * 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 * @param v The verse to check
* @return Whether we can retrieve the verse from our cache * @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 // 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<Int, String>(1000000) {
fun getId(v: Verse) = v.getOrdinal()
fun contains(v: Verse) = (this get getId(v)) != null
}