Daniel, Glad to see you are doing some rethinking / refactoring of VerseKey. Thanks. The getChaptersInBook and getVersesInChapter will be a welcome replacement for setPosition(POS_MAXVERSE) and setPosition(POS_MAXCHAPTER)
While you have "Vulcan mind-meld" with the VerseKey class, perhaps you could consider some repartitioning /decoupling of the class itself. My understanding as I look at the code, is that VerseKey references localemgr, which references swmgr, which references swmodule, and then it "spreads out" further and further with "octopus tentacles" so that there is an enormous amount of class coupling that seems less than what was intended for VerseKey. My impression is that the purpose of VerseKey itself is rather modest ... to provide KJV versification so an application can get the information that there are 50 chapters in Genesis, and that Gen 1:31 is the last verse in Gen 1, for example. Also, it can "work very closely with" rawtext, rawverse, zverse, and ztext to "unpack" the offsets and lengths of the actual verses in the compressed and uncompressed files. My limited experience with building some of the test examples in the test subdirectory is that if you have VerseKey.h, your code will end up needing to get such seemingly unrelated classes as osis to be able to link. Is that amount of class coupling necessary? Perhaps now is a reasonable time to consider some class decoupling? My impression is that there could be a class from which the current VerseKey "is-a" derived, which is concerned strictly with versification? Or VerseKey "has-a" member variable which is a class which accomplishes this functionality? Perhaps a goal to shoot for ... VerseKey is decoupled enough such that with only rawtext, rawverse, zverse, and ztext, it can be compiled/linked and still accomplish a significant amount of diatheke functionality ... something that might be called "MiniDiatheke"? ----- Original Message ----- From: "Daniel Glassey" <[EMAIL PROTECTED]> To: "SWORD Developers' Collaboration Forum" <[email protected]> Sent: Monday, June 20, 2005 12:07 PM Subject: [sword-devel] Re: proposed changes to VerseKey > On 18/06/05, Daniel Glassey <[EMAIL PROTECTED]> wrote: > > At the moment VerseKey exposes a couple of implementation variables as > > public variables. > > BMAX, books, builtin_BMAX, builtin_books > > > > I'd like to make them private and make member functions to expose these. > > getBookName (already there) > > getBookAbbrev (already there) > > getChaptersInBook > > getVersesInChapter > > > > For the latter 2 would the frontends prefer that they return for the > > current book/chapter or that you pass in testament/book/chapter that > > you want the values for? > > > > I'd also llike to make > > setBookAbbrevs and setBooks private as they are implementation > > functions used by SWLocale > > > > SWLocale will be a friend class to VerseKey so it can access these > > internal bits. > > > > This will allow changes to be made to the internals of VerseKey > > without needing major changes in the frontends. However, the frontends > > will need to be modified slightly to use the new API funcs rather than > > the internal variables. > > > > I think GS and BT only use them to get the book names for populating > > the books combobox. > > BibleCS only has 1 line that needs changed in mainfrm.cpp - I think to > > use getBookName > > btw if there are no objections I'll commit that later in the week. > > I've got a couple more proposals. > > I'd like to add NewBook() as a replacement for Book(). It will return > the total book number within the Bible (i.e. Book()+39 for New > Testament books). Er, maybe that should be Book()+40 to allow for a > New Testament preface. > > Book() will remain in the API for the next release but will be > replaced by get/setBook which will use NewBook in the api rename > scheduled for 1.5.40 > http://www.crosswire.org/bugs/browse/API-2 > and then Book() will be removed for 1.5.90 in the cleanup > http://www.crosswire.org/bugs/browse/API-7 > > Would frontends be happy with that? > > I'd also like to deprecate Testament() > To replace it I'd like to add a new function isInRange > There would be some standard ranges like 'Old Testament', 'New > Testament', 'Pentateuch' (ideas for other standard ranges would be > good) > To check if the key is in the Old Testament you would do something > like key->isInRange(OLD_TESTAMENT) > > Implementation details are to be decided - should it take a const > char*, a VerseKey(range) or should both be implemented? > > Again, Testament will hang around for a bit but frontends should aim > towards not using using it. > > Regards, > Daniel > > _______________________________________________ > sword-devel mailing list: [email protected] > http://www.crosswire.org/mailman/listinfo/sword-devel > Instructions to unsubscribe/change your settings at above page > _______________________________________________ sword-devel mailing list: [email protected] http://www.crosswire.org/mailman/listinfo/sword-devel Instructions to unsubscribe/change your settings at above page
