[sword-devel] Re: proposed changes to VerseKey

[Lynn Allan]

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" <dglassey at gmail.com>
To: "SWORD Developers' Collaboration Forum"
<sword-devel at crosswire.org>
Sent: Monday, June 20, 2005 12:07 PM
Subject: [sword-devel] Re: proposed changes to VerseKey


> On 18/06/05, Daniel Glassey <dglassey at gmail.com> 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: sword-devel at crosswire.org
> http://www.crosswire.org/mailman/listinfo/sword-devel
> Instructions to unsubscribe/change your settings at above page
>