|
From: David T. <dt...@re...> - 2016-01-11 14:09:56
|
Hello,
lately I've been thinking about librevenge::RVNGString::Iter (after
fixing a bug in last()). I see several problems with it:
* It exposes it's members, rather than using pimpl like all the other
public classes. This makes it impossible to do changes to it without
breaking ABI.
* There are 2 ways to use it correctly:
librevenge::RVNGString::Iter iter(str);
iter.rewind();
while (iter.next())
handle(iter());
for (librevenge::RVNGString::Iter iter(str); !iter.last(); iter.next())
handle(iter());
* Neither of these 2 is obvious without reading the code.
* It copies the string, incurring a performance cost. (I assume this is
to avoid lifetime management issues. But it penalizes the typical use
case, not the atypical one.)
As minimum, all these problems should be addressed for librevenge 0.1.0.
But I propose to go even further and replace the current iteration
scheme by C++-style iterator. This would immediately fix the usage
problems, as every C++ programmer should be familiar with it. It would
also allow to iterate RVNGStrings with C++ range-based for loop, like
for (const char *utf8char: str)
handle(utf8char);
The old Iter interface should continue to be available for some time
(either till the release of 0.1.0 or even for the whole life time of
0.1), just hidden behind a macro (e.g.,
LIBREVENGE_ENABLE_LEGACY_ITERATORS), so it wouldn't be necessary to
rewrite all code immediately.
Of course, the other 2 iterators (RVNGPropertyList::Iter and
RVNGPropertyListVector::Iter) should be rewritten too. (And
RVNGStringVector should probably get an iterator interface as well.)
Thoughts? Opinions?
D.
|