[jsword-devel] Bookset

[DM Smith]

I've just checked in a fix for this. The other problem I saw was an internal call to getBooks() which created a copy of books. I changed it to use books directly.

It continues the assumption that front-ends have that a book by name or initials cannot appear twice. This is not quite true.

Testing it after the change Bible Desktop with 550 Books loads in less than 3 seconds. I didn't time it before hand.

Internally, I used TreeSet as a replacement for BookSet. If we can push off the sorting to the front-end, I think this can just use a HashSet. But I didn't go that far.

Let me know how it works for you.

I also did all of the outstanding pull requests.

In Him,
	DM

On Jan 26, 2014, at 2:24 PM, Chris Burrell <chris at burrell.me.uk> wrote:

> Key thingfor me is to have an O (1) lookup time. We could key by categories name and initials if it makes our easier.
> 
> A Set gives the impression you have fast insertion and contains operations
> 
> On 26 Jan 2014 18:02, "DM Smith" <dmsmith at crosswire.org> wrote:
> I'm trying to figure out how to change this so that it works for all. Since it is ingrained in the Book installer of Bible Desktop, having been designed for it, it may take a bit. I should be able to start today.
> -- DM
> 
> On Jan 25, 2014, at 10:24 PM, DM Smith <dmsmith at crosswire.org> wrote:
> 
> >
> > On Jan 25, 2014, at 4:41 PM, Chris Burrell <chris at burrell.me.uk> wrote:
> >
> >> Hi
> >>
> >> I'm looking to refactor Books.installed().getBook(name) because it takes too long when you need to look up books multiple times (and don't have an easy way of caching the JSword lookup). It's particular slow when you have 200+ resources (our server will have). This will also be more prevalent in Android where method calls are quite expensive.
> >>
> >> In STEP we always have the initials of the module (the user selects by name/initials/STEP name in the browser, where it always gets translated to initials well before it hits JSword). Do any other frontends use the getBook(name) by name?
> >>
> >> I want to at least provide way of getting the book directly from its initials. As part of this, we can several things:
> >>
> >> In Books:
> >> - refactor the getBook() method to not search first against the name, then against name insensitive and then against the initials in the BMD, and then against the initials directly
> > Feel free to change the order. BibleDesktop uses the name of the Book, not the initials.
> >
> > There's a bit of history here that's not necessarily pertinent. At one point, we included the repository name in the name of the Book to disambiguate a module that might be in two repositories. E.g. CrossWire main and CrossWire beta. Or CrossWire main and the NET site.
> >
> > This allowed two different versions of the same module. The repository name is not part of it any more.
> >
> >
> >> - If the above is not possible then at least provide a getBookByInitials (which would just look up the initials against their lower case value.
> >>
> >> In BookSet:
> >> - I can't work out why we're sorting the inserts in add(). They cause unecessary copies of the ArrayList contents in the creation of it.
> >> - I can't work out why it also implements Set. Especially, since contains would be a good candidate for using
> > I don't think you finished the sentence.
> >
> > The BookSet implements both List and Set. It implements List because it works better when tying it to a list or tree in Swing. It keeps it sorted so that it makes sense to a user. It implements Set because a Book cannot be in the list twice. Note that this is determined by compareTo, equals and hashcode on BookMetaData which uses category, initials and name in that order.
> >
> > If Set is not on the class the class has the wrong contract. It is merely a marker. The contract is maintained by the add and addAll methods.
> >
> > Regarding add. The contract of the class is that it maintains the books in sorted order (category, initials, name). That has to be invariant. The method addAll has a comment that it probably shouldn't call add for all the members but rather stuff them all in and then sort the array.
> >
> >
> >>
> >>
> >> So there are two options really:
> >> - change BookSet to be based on a Map. getBooksByInitials would use the map directly. getBook would iterate through the contents (or key the contents in a separate map for faster access)
> >> - Add a map to BookSet to cache the lookups
> >
> > I think the second would be fine. However, it will be tough to maintain. It has to handle collisions properly. Currently, the class allows duplicates by initials or by name or by name and initials (provided that they are in different categories.) During lookup, it grabs the first that it finds. Not the last. So if there's a dictionary and a Bible both with the initials KJV, it will find the KJV Bible because the Biblical Material category comes before Dictionary category. If you merely do it based upon insertion order, there's no telling what you'll get.
> >
> > The example is made up, but these things have happened.
> >
> > Give a real possibility. A module makes it into the Xiphos repository and later into the CrossWire repository. Same initials and same name. Likewise for Beta and main. For example, there is a KJV module in the Beta repository and also in the main repository.
> >
> >>
> >> My preference would be to replace the BookSet implementation altogether. But the easy option would be to have a Map lookup. Do we use any of the Set<> methods?
> >
> > I'm not seeing any Set methods other than those implemented by List. The notion of Set was that it could be used in a Set context.
> >
> >> Would it make sense to replace BookSet with a LinkedHashMap?
> >
> > How would that maintain natural ordering? (category, initials, name)
> >
> >>
> >>
> >> Chris
> >>
> >> _______________________________________________
> >> jsword-devel mailing list
> >> jsword-devel at crosswire.org
> >> http://www.crosswire.org/mailman/listinfo/jsword-devel
> >
> > _______________________________________________
> > jsword-devel mailing list
> > jsword-devel at crosswire.org
> > http://www.crosswire.org/mailman/listinfo/jsword-devel
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.crosswire.org/pipermail/jsword-devel/attachments/20140127/046c07ba/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4145 bytes
Desc: not available
URL: <http://www.crosswire.org/pipermail/jsword-devel/attachments/20140127/046c07ba/attachment.p7s>