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. |