From 1eb914819d5a22c6ecc129a20acf2577a26bddc2 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Fri, 24 Oct 2014 23:53:39 -0400 Subject: [PATCH] Refactor main menu to Kotlin So much less code to write and maintain... love it. --- .../downloader/manager/InstalledManager.java | 9 +- .../navigation/ExpListNavAdapter.java | 185 ------------------ .../activity/viewer/BibleViewerModules.java | 13 +- .../viewer/ExpListNavDrawerFragment.java | 54 +---- .../exception/NoBooksInstalledException.java | 7 + .../minimalbible/activity/viewer/BibleMenu.kt | 84 ++++++++ .../activity/viewer/BibleViewClient.kt | 2 +- .../service/manager/BookManager.kt | 14 +- .../org/crosswire/jsword/book/BookUtil.kt | 28 +++ .../jsword/versification/VersificationUtil.kt | 68 ++----- 10 files changed, 146 insertions(+), 318 deletions(-) delete mode 100644 app/src/main/java/org/bspeice/minimalbible/activity/navigation/ExpListNavAdapter.java create mode 100644 app/src/main/java/org/bspeice/minimalbible/exception/NoBooksInstalledException.java create mode 100644 app/src/main/kotlin/org/bspeice/minimalbible/activity/viewer/BibleMenu.kt create mode 100644 app/src/main/kotlin/org/crosswire/jsword/book/BookUtil.kt diff --git a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/manager/InstalledManager.java b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/manager/InstalledManager.java index 76fa8de..1baa8b4 100644 --- a/app/src/main/java/org/bspeice/minimalbible/activity/downloader/manager/InstalledManager.java +++ b/app/src/main/java/org/bspeice/minimalbible/activity/downloader/manager/InstalledManager.java @@ -14,8 +14,6 @@ import java.util.List; import javax.inject.Inject; import javax.inject.Singleton; -import rx.Observable; - /** * Manager to keep track of which books have been installed */ @@ -23,6 +21,7 @@ import rx.Observable; public class InstalledManager implements BooksListener { @Inject Books installedBooks; + // TODO: Why is this injected if we initialize in the constructor? @Inject List installedBooksList; private String TAG = "InstalledManager"; @@ -38,12 +37,6 @@ public class InstalledManager implements BooksListener { return installedBooksList.contains(b); } - public Observable getInstalledBooks() { - // This method is needed to provide a fresher copy of what's installed - // than Books.getInstalled() does. - return Observable.from(installedBooksList); - } - @Override public void bookAdded(BooksEvent booksEvent) { Log.d(TAG, "Book added: " + booksEvent.getBook().toString()); diff --git a/app/src/main/java/org/bspeice/minimalbible/activity/navigation/ExpListNavAdapter.java b/app/src/main/java/org/bspeice/minimalbible/activity/navigation/ExpListNavAdapter.java deleted file mode 100644 index c8d400c..0000000 --- a/app/src/main/java/org/bspeice/minimalbible/activity/navigation/ExpListNavAdapter.java +++ /dev/null @@ -1,185 +0,0 @@ -package org.bspeice.minimalbible.activity.navigation; - -import android.content.Context; -import android.view.LayoutInflater; -import android.view.View; -import android.view.ViewGroup; -import android.widget.BaseExpandableListAdapter; -import android.widget.TextView; - -import org.bspeice.minimalbible.R; - -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import butterknife.ButterKnife; -import butterknife.InjectView; - -/** - * ExpandableListView Navigation Drawer - * T1 represents Group objects, T2 is child objects - * Not sure if I'll ever actually need to re-use this, but go ahead and make it generic. - * TODO: Document this. - */ -public class ExpListNavAdapter extends BaseExpandableListAdapter { - - // Now we could technically implement this structure using a LinkedHashMap, but - // it's easier both to understand and program if we implement this using two maps. - Map indexableBibleBooks; - Map> chaptersForBook; - - private int groupHighlighted; - private int childHighlighted; - - public ExpListNavAdapter(List groups, Map> children) { - - // Let the map know ahead of time how big it will be - // int bookCount = versification.getBookCount(); - indexableBibleBooks = new HashMap(groups.size()); - chaptersForBook = new HashMap>(groups.size()); - - // Is it terrible that I don't like using an actual for loop? - for (int index = 0; index < groups.size(); index++) { - T1 gItem = groups.get(index); - indexableBibleBooks.put(index, gItem); - chaptersForBook.put(gItem, children.get(gItem)); - } - } - - @Override - public int getGroupCount() { - return indexableBibleBooks.size(); - } - - @Override - public int getChildrenCount(int i) { - return chaptersForBook.get(indexableBibleBooks.get(i)).size(); - } - - @Override - public T1 getGroup(int i) { - return indexableBibleBooks.get(i); - } - - /** - * @param i The group position - * @param i2 The child position - * @return The child chapter value - */ - @Override - public T2 getChild(int i, int i2) { - return chaptersForBook.get(indexableBibleBooks.get(i)).get(i2); - } - - @Override - public long getGroupId(int i) { - return i; - } - - @Override - public long getChildId(int i, int i2) { - return i2; - } - - @Override - public boolean hasStableIds() { - return true; - } - - /** - * Get the views for this group - * - * @param position - * @param expanded - * @param convertView - * @param parent - * @return - */ - @Override - public View getGroupView(int position, boolean expanded, - View convertView, ViewGroup parent) { - NavItemHolder bookHolder; - if (convertView == null || convertView.getTag() == null) { - LayoutInflater inflater = (LayoutInflater) parent.getContext() - .getSystemService(Context.LAYOUT_INFLATER_SERVICE); - convertView = inflater.inflate(R.layout.list_navigation_drawer, - parent, false); - bookHolder = new NavItemHolder(convertView); - convertView.setTag(bookHolder); - } else { - bookHolder = (NavItemHolder) convertView.getTag(); - } - - bookHolder.bind(getGroup(position), position == groupHighlighted); - return convertView; - } - - /** - * Get the view for a child - * - * @param groupPosition - * @param childPosition - * @param isLastChild - * @param convertView - * @param parent - * @return - */ - @Override - public View getChildView(int groupPosition, int childPosition, - boolean isLastChild, View convertView, ViewGroup parent) { - NavItemHolder chapterHolder; - if (convertView == null || convertView.getTag() == null) { - LayoutInflater inflater = (LayoutInflater) parent.getContext() - .getSystemService(Context.LAYOUT_INFLATER_SERVICE); - convertView = inflater.inflate(R.layout.list_navigation_drawer, - parent, false); - chapterHolder = new NavItemHolder(convertView); - convertView.setTag(chapterHolder); - } else { - chapterHolder = (NavItemHolder) convertView.getTag(); - } - - chapterHolder.bind(getChild(groupPosition, childPosition), - childPosition == childHighlighted); - return convertView; - } - - @Override - public boolean isChildSelectable(int i, int i2) { - return true; - } - - public void setGroupHighlighted(int groupHighlighted) { - this.groupHighlighted = groupHighlighted; - } - - public void setChildHighlighted(int childHighlighted) { - this.childHighlighted = childHighlighted; - } - - /** - * Class to hold elements for the navbar - doesn't matter if they're group or child. - * T3 is either T1 or T2, doesn't matter. - */ - class NavItemHolder { - @InjectView(R.id.navlist_content) - TextView content; - - View v; - - public NavItemHolder(View v) { - this.v = v; // Needed for resolving colors below - ButterKnife.inject(this, v); - } - - public void bind(T3 object, boolean highlighted) { - content.setText(object.toString()); - if (highlighted) { - content.setTextColor(v.getResources().getColor(R.color.navbar_highlight)); - } else { - content.setTextColor(v.getResources().getColor(R.color.navbar_unhighlighted)); - } - } - } -} \ No newline at end of file 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 e7b860b..973c03d 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,11 +2,9 @@ package org.bspeice.minimalbible.activity.viewer; import android.util.Log; -import org.bspeice.minimalbible.activity.navigation.ExpListNavAdapter; import org.bspeice.minimalbible.service.book.VerseLookupModules; import org.bspeice.minimalbible.service.manager.BookManager; import org.crosswire.jsword.book.Book; -import org.crosswire.jsword.versification.VersificationUtil; import java.util.NoSuchElementException; import java.util.concurrent.atomic.AtomicReference; @@ -21,17 +19,17 @@ import rx.functions.Action1; import rx.functions.Func1; /** - * Created by bspeice on 6/18/14. + * Modules used for the BibleViewer activity */ @Module( injects = { BibleViewer.class, BookFragment.class, - ExpListNavDrawerFragment.class, - ExpListNavAdapter.class + ExpListNavDrawerFragment.class }, includes = VerseLookupModules.class ) +@SuppressWarnings("unused") public class BibleViewerModules { BibleViewer activity; @@ -98,9 +96,4 @@ public class BibleViewerModules { BookManager bookManager() { return new BookManager(); } - - @Provides - VersificationUtil provideVersificationUtil() { - return new VersificationUtil(); - } } \ No newline at end of file diff --git a/app/src/main/java/org/bspeice/minimalbible/activity/viewer/ExpListNavDrawerFragment.java b/app/src/main/java/org/bspeice/minimalbible/activity/viewer/ExpListNavDrawerFragment.java index 8b8af8c..6aae851 100644 --- a/app/src/main/java/org/bspeice/minimalbible/activity/viewer/ExpListNavDrawerFragment.java +++ b/app/src/main/java/org/bspeice/minimalbible/activity/viewer/ExpListNavDrawerFragment.java @@ -8,22 +8,12 @@ import android.widget.ExpandableListView; import org.bspeice.minimalbible.Injector; import org.bspeice.minimalbible.R; -import org.bspeice.minimalbible.activity.navigation.ExpListNavAdapter; import org.bspeice.minimalbible.activity.navigation.NavDrawerFragment; import org.crosswire.jsword.book.Book; -import org.crosswire.jsword.versification.BibleBook; -import org.crosswire.jsword.versification.VersificationUtil; - -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; import javax.inject.Inject; import javax.inject.Named; -import rx.functions.Action1; - /** * ExpandableListView for managing books of the Bible. * We extend from @link{BaseNavigationDrawerFragment} so we can inherit some of the lifecycle @@ -34,7 +24,6 @@ import rx.functions.Action1; */ public class ExpListNavDrawerFragment extends NavDrawerFragment { - @Inject VersificationUtil vUtil; @Inject @Named("MainBook") Book mainBook; @@ -48,48 +37,7 @@ public class ExpListNavDrawerFragment extends NavDrawerFragment { mActualListView = (ExpandableListView) inflater.inflate( R.layout.fragment_expandable_navigation_drawer, container, false); - /* - mDrawerListView - .setOnItemClickListener(new AdapterView.OnItemClickListener() { - @Override - public void onItemClick(AdapterView parent, View view, - int position, long id) { - selectItem(position); - } - }); - */ - - List bibleBooks; - if (mainBook != null) { - bibleBooks = vUtil.getBookNames(mainBook) - .toList().toBlocking().first(); - } else { - bibleBooks = new ArrayList(); - } - - - // I really don't like how we build the chapters, but I'm not adding Guava just for Range. - // This isn't a totally functional style map-reduce, but the reduce step is - // unnecessarily verbose. Like this comment. - final Map> chapterMap = new HashMap>(); - if (mainBook != null) { - vUtil.getBooks(mainBook).forEach(new Action1() { - @Override - public void call(BibleBook bibleBook) { - int bookCount = vUtil.getChapterCount(mainBook, bibleBook); - List chapterList = new ArrayList(bookCount); - for (int i = 0; i < bookCount; i++) { - chapterList.add(i + 1); // Index to chapter number - } - chapterMap.put(vUtil.getBookName(mainBook, bibleBook), chapterList); - } - }); - - ExpListNavAdapter adapter = - new ExpListNavAdapter(bibleBooks, chapterMap); - - mActualListView.setAdapter(adapter); - } + mActualListView.setAdapter(new BibleMenu(mainBook)); mActualListView.setItemChecked(mCurrentSelectedPosition, true); return mActualListView; diff --git a/app/src/main/java/org/bspeice/minimalbible/exception/NoBooksInstalledException.java b/app/src/main/java/org/bspeice/minimalbible/exception/NoBooksInstalledException.java new file mode 100644 index 0000000..a77b0de --- /dev/null +++ b/app/src/main/java/org/bspeice/minimalbible/exception/NoBooksInstalledException.java @@ -0,0 +1,7 @@ +package org.bspeice.minimalbible.exception; + +/** + * Error to be thrown when no books are currently installed. + */ +public class NoBooksInstalledException extends Exception { +} diff --git a/app/src/main/kotlin/org/bspeice/minimalbible/activity/viewer/BibleMenu.kt b/app/src/main/kotlin/org/bspeice/minimalbible/activity/viewer/BibleMenu.kt new file mode 100644 index 0000000..d8dbd51 --- /dev/null +++ b/app/src/main/kotlin/org/bspeice/minimalbible/activity/viewer/BibleMenu.kt @@ -0,0 +1,84 @@ +package org.bspeice.minimalbible.activity.viewer + +import android.widget.BaseExpandableListAdapter +import android.view.View +import android.view.ViewGroup +import org.crosswire.jsword.book.Book +import org.crosswire.jsword.book.getVersification +import org.crosswire.jsword.versification.getBooks +import org.crosswire.jsword.book.bookName +import android.widget.TextView +import org.bspeice.minimalbible.R +import android.content.Context +import android.view.LayoutInflater + +/** + * Created by bspeice on 10/24/14. + */ + +class BibleMenu(val b: Book) : BaseExpandableListAdapter() { + + // Map BibleBooks to the number of chapters they have + val menuMappings = b.getVersification().getBooks().map { + Pair(it, b.getVersification().getLastChapter(it)) + } + + var groupHighlighted: Int = 0 + var childHighlighted: Int = 0 + + override fun getGroupCount(): Int = menuMappings.count() + + override fun getChildrenCount(group: Int): Int = menuMappings.elementAt(group).component2() + + override fun getGroup(group: Int): String = + b.bookName(menuMappings.elementAt(group).component1()) + + override fun getChild(group: Int, child: Int): Int = child + 1 // Index offset + + override fun getGroupId(group: Int): Long = group.toLong() + + override fun getChildId(group: Int, child: Int): Long = child.toLong() + + override fun hasStableIds(): Boolean = true + + override fun isChildSelectable(group: Int, child: Int): Boolean = true + + private fun doBinding(convertView: View?, parent: ViewGroup?, + obj: Any, highlight: Boolean): View { + val finalView: View = if (convertView != null) convertView + else { + val inflater = parent!!.getContext() + .getSystemService(Context.LAYOUT_INFLATER_SERVICE) as LayoutInflater + inflater.inflate(R.layout.list_navigation_drawer, parent, false) + } + + val holder: NavItemHolder = if (finalView.getTag() != null) finalView.getTag() as NavItemHolder + else NavItemHolder(finalView, R.id.navlist_content) + + holder.bind(obj, highlight) + finalView.setTag(holder) + return finalView + } + + override fun getGroupView(position: Int, expanded: Boolean, + convertView: View?, parent: ViewGroup?): View = + doBinding(convertView, parent, getGroup(position), position == groupHighlighted) + + override fun getChildView(group: Int, child: Int, isLast: Boolean, + convertView: View?, parent: ViewGroup?): View = + doBinding(convertView, parent, getChild(group, child), child == childHighlighted) + + // Resource should be IdRes + class NavItemHolder(val bindTo: View, resource: Int) { + val content: TextView = bindTo.findViewById(resource) as TextView + + fun bind(obj: Any, highlighted: Boolean) { + content.setText(obj.toString()) + if (highlighted) + content.setTextColor(bindTo.getResources().getColor(R.color.navbar_highlight)) + else + content.setTextColor(bindTo.getResources().getColor(R.color.navbar_unhighlighted)) + } + } + +} \ No newline at end of file diff --git a/app/src/main/kotlin/org/bspeice/minimalbible/activity/viewer/BibleViewClient.kt b/app/src/main/kotlin/org/bspeice/minimalbible/activity/viewer/BibleViewClient.kt index 013e439..1dd0b43 100644 --- a/app/src/main/kotlin/org/bspeice/minimalbible/activity/viewer/BibleViewClient.kt +++ b/app/src/main/kotlin/org/bspeice/minimalbible/activity/viewer/BibleViewClient.kt @@ -5,10 +5,10 @@ import org.bspeice.minimalbible.service.book.VerseLookupService import android.webkit.WebViewClient import android.webkit.JavascriptInterface import org.crosswire.jsword.book.Book -import org.crosswire.jsword.versification.getVersification import java.util.ArrayList import android.util.Log import rx.subjects.PublishSubject +import org.crosswire.jsword.book.getVersification /** * Created by bspeice on 9/14/14. diff --git a/app/src/main/kotlin/org/bspeice/minimalbible/service/manager/BookManager.kt b/app/src/main/kotlin/org/bspeice/minimalbible/service/manager/BookManager.kt index eeb8510..ee275d5 100644 --- a/app/src/main/kotlin/org/bspeice/minimalbible/service/manager/BookManager.kt +++ b/app/src/main/kotlin/org/bspeice/minimalbible/service/manager/BookManager.kt @@ -5,30 +5,30 @@ import org.crosswire.jsword.book.Books import rx.functions.Action1 import org.crosswire.jsword.book.Book import rx.functions.Action0 +import org.bspeice.minimalbible.exception.NoBooksInstalledException +import javax.inject.Singleton /** * 'Open' class and values for mockito to subclass * http://confluence.jetbrains.com/display/Kotlin/Classes+and+Inheritance */ -//@Singleton +Singleton open class BookManager() { // Some extra books like to think they're installed, but trigger NPE all over the place... val ignore = array("ERen_no", "ot1nt2"); + open val installedBooks = Observable.from(Books.installed()!!.getBooks()) ?.filter { !ignore.contains(it!!.getInitials()) } - ?.cache() + ?.cache() ?: throw NoBooksInstalledException() + var refreshComplete = false; { // TODO: Cleaner way of expressing this? - installedBooks?.subscribe(Action1 { result -> }, + installedBooks.subscribe(Action1 { result -> }, Action1 { error -> }, Action0 { refreshComplete = true }) } - - fun getBooks(): Observable { - return installedBooks as Observable - } } \ No newline at end of file diff --git a/app/src/main/kotlin/org/crosswire/jsword/book/BookUtil.kt b/app/src/main/kotlin/org/crosswire/jsword/book/BookUtil.kt new file mode 100644 index 0000000..b2db842 --- /dev/null +++ b/app/src/main/kotlin/org/crosswire/jsword/book/BookUtil.kt @@ -0,0 +1,28 @@ +package org.crosswire.jsword.book + +import org.crosswire.jsword.versification.Versification +import org.crosswire.jsword.versification.system.Versifications +import android.util.Log +import org.bspeice.minimalbible.service.manager.InvalidBookException +import org.crosswire.jsword.versification.getBookNames +import org.crosswire.jsword.versification.BibleBook + +/** + * Utility methods for dealing with Books + */ + +fun Book.getVersification(): Versification { + val v = Versifications.instance()!!.getVersification( + this.getBookMetaData()!!.getProperty(BookMetaData.KEY_VERSIFICATION).toString() + ) + if (v == null) { + Log.e(javaClass().getSimpleName(), "Invalid book: " + this.getInitials()) + throw InvalidBookException(this.getInitials()) + } else + return v +} + +fun Book.bookNames(): List = this.getVersification().getBookNames() + +fun Book.bookName(bBook: BibleBook): String = + this.getVersification().getBookName(bBook).getLongName() \ No newline at end of file diff --git a/app/src/main/kotlin/org/crosswire/jsword/versification/VersificationUtil.kt b/app/src/main/kotlin/org/crosswire/jsword/versification/VersificationUtil.kt index 33068e2..2ef1094 100644 --- a/app/src/main/kotlin/org/crosswire/jsword/versification/VersificationUtil.kt +++ b/app/src/main/kotlin/org/crosswire/jsword/versification/VersificationUtil.kt @@ -1,45 +1,16 @@ package org.crosswire.jsword.versification -import org.crosswire.jsword.book.Book import java.util.ArrayList -import org.crosswire.jsword.versification.system.Versifications -import org.crosswire.jsword.book.BookMetaData -import rx.Observable -import android.util.Log -import org.bspeice.minimalbible.service.manager.InvalidBookException /** - * Created by bspeice on 9/10/14. + * VersificationUtil class allows Java to easily reach in to Kotlin */ - -class VersificationUtil() { - class object { - val INTROS = array( - BibleBook.INTRO_BIBLE, - BibleBook.INTRO_OT, - BibleBook.INTRO_NT - ) - } - - fun getBookNames(b: Book): Observable { - return Observable.from(b.getVersification().getBookNames()) - } - - fun getBooks(b: Book): Observable { - return Observable.from(b.getVersification().getBooks()) - } - - fun getChapterCount(b: Book, bibleBook: BibleBook): Int { - return b.getVersification().getChapterCount(bibleBook) - } - - fun getBookName(b: Book, bibleBook: BibleBook): String { - return b.getVersification().getLongName(bibleBook) - } - - fun getVersification(b: Book): Versification { - return b.getVersification() - } +object INTRO_BOOKS { + val INTROS = array( + BibleBook.INTRO_BIBLE, + BibleBook.INTRO_OT, + BibleBook.INTRO_NT + ) } // TODO: Refactor (is there a better way to accomplish this?) and move @@ -52,26 +23,15 @@ fun Iterator.iterable(): Iterable { return list } -fun Versification.getBooks(): List { - return this.getBookIterator()!!.iterable() - .filter { !VersificationUtil.INTROS.contains(it) } -} +fun Versification.getAllBooks(): List = + this.getBookIterator()!!.iterable().toList() -fun Versification.getBookNames(): List { - return this.getBooks().map { this.getLongName(it) } -} +fun Versification.getBooks(): List = + this.getAllBooks().filter { !INTRO_BOOKS.INTROS.contains(it) } + +fun Versification.getBookNames(): List = + this.getBooks().map { this.getLongName(it) } fun Versification.getChapterCount(b: BibleBook): Int { return this.getLastChapter(b) -} - -fun Book.getVersification(): Versification { - val v = Versifications.instance()!!.getVersification( - this.getBookMetaData()!!.getProperty(BookMetaData.KEY_VERSIFICATION).toString() - ) - if (v == null) { - Log.e(javaClass().getSimpleName(), "Invalid book: " + this.getInitials()) - throw InvalidBookException(this.getInitials()) - } else - return v } \ No newline at end of file