[sword-devel] Re: proposed changes to VerseKey

[Daniel Glassey]

On 20/06/05, Joachim Ansorg <nospam+sword-devel at joachim-ansorg.de> wrote:
> Hi,
> 
> > > getBookName (already there)
> > > getBookAbbrev (already there)
> 
> This needs to use the i18n abbrevs.

What do you mean?
 
> > > 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 don't think these functions fit into VerseKey, this is module specific data.
> I think it should go into another class, but I'm not aware of a public Bible
> module class for that.
> I don't know.

Well, the data is currently stored in VerseKey. It is in a struct that
includes book names - SWLocale modifies it to do localised book names.
Though I agree it ought to be in the SWModule derived class instead in
the end - it is a bit more work to move it there so I think we should
wait for Troy and general consensus before making the change that way.

e.g for BT once it is using getChaptersInBook it would just need to
change from key->getChaptersInBook to module->getChaptersInBook.

> > > 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.
> 
> And to get Verse and Chapter counts I think (without looking into the code).

it's a minor change to set the book/chapter and use getChaptersInBook
etc instead

> > > 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 think Troy is offline these days, isn't he?
> I don't know what he'd like.

Good point. I'm not doing anything to change current behaviour (apart
from bugfixes and hiding of exposed internal variables), just adding
new API functions so I hope it is ok committing. They won't be fixed
in stone - can always be removed.

> > 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.
> 
> I don't like the name :)

Well, if you can come up with a better one then I'd be happy to use it ;)

> What about v11n issues with this change?

I need to look more into that. I'll let you know. The
headings/prefaces make things complicated. If it makes things harder
for the frontends it is less likely to be worth doing.

> > 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)
> 
> Hm, I think we need the Testament and book stuff to check which testament is
> available in the Bible module to just display the book available.
>
> I'd love an easy way to get the list of available books and their chapters,
> verses.

I was going to get to that with the thread about the conf file and coverage ;)
I think the broad coverage range is still a good idea for the conf
file to help the installers. (Or the function I'm about to propose
could be used to make a utility to autogenerate the conf file line.)

ListKey SWModule::covers()
This provides a ListKey that has a list of ranges that the module covers.

Though I'm not sure how to implement that - help appreciated.

You could also have bool SWModule::covers(char*/VerseKey) which takes
a range and tells you if the module covers it or not. It would be able
to use the same standard ranges as isInRange.

(As usual I'm not good at coming up with these names so please suggest
better ones while the names are fluid).

> > Implementation details are to be decided - should it take a const
> > char*, a VerseKey(range) or should both be implemented?
> 
> I'd like both.

Sounds good to me.

Thanks,
Daniel