2010/10/23 Fridrich Strba <fridrich.st...@bluewin.ch>:
> Will,
>
> My first wild idea is that if you do it all properly, go for it. Like to
> be sure that we don't require user to free anything she did not allocate
> and that we don't return reference to a temporary from a function, try
> it.

const char * implies read-only, just like const WPXString &. I don't
think there's any risk of memory issues here, as long as the API is
used as intended. :)

> Now, concerning the binding, we don't have big issues with binding
> libwpd's WPXString in the libwpd2-bindings module and also the len()
> function that is returning the unicode character count is great.

Hmm, I didn't think about that. Looking closely at WriterPerfect, I
guess the main reason we have the extra functions to return the length
of the string is to allow us to iterate over it in the case that we're
implementing a document listener which requires us to explicitly
delineate spaces (e.g. opendocument, html, ....)... something that,
come to think of it, we should really be taking care of inside libwpd
by providing an insertSpace function. :)

> So, if it is not really a huge performance problem I would plead to
> stick with WPXString for 0.9.x and try to simplify the API after. The
> reason is that I actually settled for using the WPXString everywhere in
> libraries that have something to do with libwpd and would not be very
> confortable to do such a huge change at this point of the release cycle.

The main reason to do this is definitely API ease of use, not
performance. I imagine this change will improve performance somewhat
(because we'll be copying fewer strings), but that's never really been
an issue with libwpd (libwpd can process even a large document in a
fraction of a second).

The thing is, if we don't do this now, it will be a long time before
we ever do (since it's an API-breaking change), so we'll be stuck with
the current API for a long time. Really, in almost all cases the
conversion is very straightforward so I think the risk is quite low.
Take this function from the raw listener:

void RawDocumentGenerator::insertText(const WPXString &text)
{
        __iprintf("insertText(text: %s)\n", text.cstr());
}

After the conversion, the function would look like this:

void RawDocumentGenerator::insertText(const char *text)
{
        __iprintf("insertText(text: %s)\n", text);
}

I imagine I could convert everything libwpd-related (libwpd,
writerperfect, bindings, libwpg, the abiword plugin) to this approach
(with the insertSpace function I mentioned above) in the space of an
afternoon next week.
-- 
William Lachance
wrl...@gmail.com

------------------------------------------------------------------------------
Nokia and AT&T present the 2010 Calling All Innovators-North America contest
Create new apps & games for the Nokia N8 for consumers in  U.S. and Canada
$10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing
Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store 
http://p.sf.net/sfu/nokia-dev2dev
_______________________________________________
Libwpd-devel mailing list
Libwpd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libwpd-devel

Reply via email to