[Libreoffice] [PATCH] Changed String to OUString in funcdesc.hxx
Kohei Yoshida
kyoshida at novell.com
Mon Jan 10 08:17:43 PST 2011
On Sun, 2011-01-09 at 21:41 +0000, Caolán McNamara wrote:
> On Sun, 2011-01-09 at 20:37 +0100, Soeren Moeller wrote:
> > And here another patch for sc/inc/funcdesc.hxx and related source
> > files,
> ...
> >(using casts when forced to use String originally because of ResId).
>
> Kohei will probably want to have a look at these, but as an aside, how
> about adding something like
>
> static rtl::OUString toString(const ResId&)
>
> to the tools resid.hxx ResId class and hide the nasty convert cast
> kludge in the impl of that, which could be tucked away in the old
> tools/source/strucvt.cxx for now.
Yeah, that would make the transition a bit smoother. We get String
instances from ResId in many places, and having a function like that
would indeed hide the ugliness in one place.
The alternative is to use String to create a temporary string instance
from (Sc)ResId, then pass it on to the constructor of rtl::OUString.
But that always adds the overhead of instantiating two string instances
instead of one. Even with that overhead; however, I prefer that over
casting String to OUString as long as the code is not
performance-critical. But Caolan's suggestion would probably be the
best approach given the current constraints surrounding (Sc)ResId,
String, and OUString.
BTW, a minor nitpick to Soeren's original patch. In GetParamList() and
getSignature(), you could consider using rtl::OUStringBuffer instead of
rtl::OUString for better performance. rtl::OUString is by design
immutable, and it creates a new string instance every time you add a new
string segment to it. The String class OTOH is not immutable hence it
is sometimes used as a string buffer. Looking at the way it is used in
GetParamList() and getSignature(), rtl::OUStringBuffer probably is a
better replacement candidate.
Kohei
--
Kohei Yoshida, LibreOffice hacker, Calc
<kyoshida at novell.com>
More information about the LibreOffice
mailing list