Re: [Libreoffice] [PATCH] Changed String to OUString in funcdesc.hxx

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@novell.com>