Abdelrazak Younes wrote:
Richard Heck wrote:
Attached is an updated patch. It's long, to be sure, but not really
that complicated. A lot of it is really code re-organization that was
made possible (and necessary) by the fact that we now have
full-fledged classes where once there was just a collection of
functions. So a great lot of this can really be ignored: Biblio.cpp
and Buffer*, and almost nothing happens in the insets/* files,
either. (There are a lot of changes in insets/InsetCitation.cpp, but
these are almost all re-arranging of code---quite a lot of stuff from
the biblio namespace was used only here and so got moved---and simple
adaptations to other changes.) So if you want to look at this, look
at the definitions of the new classes in Biblio.h and then jump ahead
to line 1262. The action is in QCitation*.
The reorganisation looks OK AFAICS, I don't have the time to look at
the details and won't in any case because I am mostly ignorant about
BibTeX.
Thanks for looking.
[snip]
You can return a const ref here:
docstring const BibTeXInfo::getValueForField(docstring const & field)
const
{
BibTeXInfo::const_iterator it = find(field);
if (it != end())
return it->second;
static docstring const empty_value;
return empty_value;
}
Thanks. I've often wondered how to do that, since you can't "return
docstring()" in this case.
+docstring const BibTeXInfo::getValueForField(string const & field) const
+{
+ return getValueForField(from_ascii(field));
+}
Ditto, but why is there a need for this method?
It's just a convenience. The field names are stored as docstring, but we
occasionally need to do this: getValueForField("author"). So it makes
things slightly more compact.
Also are we sure that we don't need unicode keys for the BibKeyList?
If no, then I'd suggest to just use that:
std::map<docstring, BibTeXInfo>
I was somewhat confused about this. The old code used ASCII, but the
comments say you don't have to do so. Anyway, I'll change it to
docstring. That has to be the right thing to do.
About the the std::map inheritance, you have to ask yourself if you
really need the different map methods. If yes, then methods like
getValueForField are not really needed as you use the map operators
directly.
Actually, it is, because operator[] has no const form.
But, AFAIU, we only need some const access methods so it might be
better to just put the actual map in a private member. This has the
advantage that we can change the internal structure without changing
the public interface. Something like that.
I did it that way initially, and found myself adding more and more map
methods to my wrapper. So I gave up and did the inheritance.
I would also personally simplify the access method names. Instead of
offering different access method for fields, entry, I'd suggest to
just use the interface in BibTeXInfo:
getAbbreviatedAuthor(key) -> item[key].getAbbreviatedAuthor();
As you can see, I am not a big fan of redirection. I prefer explicit
rather than implicit. This facilitate further reorganization if need be.
I thought about this, and then thought about it some more, and I'll
think about it yet more. Part of the problem, again, is that you can't
do item[key] in most of these cases, again because of the lack of a
const operator[]. Which is a real pain, as you can see.
Richard
--
==================================================================
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==================================================================
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto
