From Eike Rathke <erack@redhat.com>:
Eike Rathke has posted comments on this change.
Change subject: Replaced deprecated tools/String with OUString in ScAddInCol
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(15 inline comments)
Please do the changes lined out, thanks.
Sorry for nitpicking ;-)
....................................................
File sc/source/core/tool/addincol.cxx
Line 462: aFuncName += ::rtl::OUString( pFuncNameArray[nFuncPos] );
Just a hint: these operator+= lines create temporary OUString instances (actually two per statement
in this case) de/allocating memory that could be avoided by using an OUStringBuffer of a sufficient
initial size (i.e. length of aServiceName plus '.' plus length of pFuncNameArray[nFuncPos]) and use
OUStringBuffer::append() to concatenate and a final OUStringBuffer::makeStringAndClear() call to
create the OUString. However, I think it doesn't matter that much here, just to make you aware of
how that could be done.
Line 643: xub_StrLen nPos = aFullName.lastIndexOf( (sal_Unicode) '.' );
The xub_StrLen (defined to sal_uInt16) is wrong now, OUString positions such as returned by
lastIndexOf() are sal_Int32 instead.
Line 644: if ( nPos != STRING_NOTFOUND && nPos > 0 )
So that means to also change comparisons with STRING_NOTFOUND (which is defined to 0xFFFF),
lastIndexOf() returns -1 if not found, so the condition here can be simplified to
if (nPos > 0)
Line 934: aLocalU = rtl::OUString("###");
This should not need another temporary OUString anymore (not related to your changes, proper
operator=() have been implemented meanwhile) and instead simply be
aLocalU = "###";
Line 946: aDescU = rtl::OUString("###");
Same here,
aDescU = "###";
Line 948: ::rtl::OUString aDescription = ::rtl::OUString(
aDescU );
This can simply be
::rtl::OUString aDescription( aDescU );
Line 971: aArgName = rtl::OUString("###");
aArgName = "###";
Line 981: aArgName = rtl::OUString("###");
aArgName = "###";
Line 990: aDesc.aDescription = ::rtl::OUString(
aArgDesc );
Temporary conversion ctors are unnecessary here now, so
aDesc.aName = aArgName;
aDesc.aDescription = aArgDesc;
Line 1165: aDesc.aName = aDesc.aDescription =
::rtl::OUString::createFromAscii( "###" );
aDesc.aName = aDesc.aDescription = "###";
Line 1319: if (!aDesc.getLength())
For this !...getLength() we now have isEmpty(), so
if (aDesc.isEmpty())
Line 1609: aString = ::rtl::OUString( aUStr );
As the aString member variable now is of type OUString the temporary isn't needed anymore, so this
rtl::OUString aUStr;
rNewRes >>= aUStr;
aString = ::rtl::OUString( aUStr );
instead can simply be
rNewRes >>= aString;
....................................................
File sc/source/core/tool/compiler.cxx
Line 5199: rName = String(prName);
In our convention a leading small p indicates a pointer, this should be aName instead of prName.
There's also a converting String::operator=(const OUString&) so this temporary String can be
simplified to
::rtl::OUString aName(rName);
ScGlobal::GetAddInCollection()->LocalizeString( aName );
rName = aName;
Line 5245: aEntry.Name = String(aName);
Same here,
aEntry.Name = aName;
is sufficient, with the additional benefit that once we also converted aEntry.Name to OUString
there wouldn't be a superfluous automatic back and forth conversion.
....................................................
File sc/source/filter/excel/xiroot.cxx
Line 282: return String(aScName);
Using the automatic conversion should work here
return aScName;
--
To view, visit https://gerrit.libreoffice.org/258
To unsubscribe, visit https://gerrit.libreoffice.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7059f10617b9a33ba63690c980b96d95d9023c55
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: master
Gerrit-Owner: Sören Möller <soerenmoeller2001@gmail.com>
Gerrit-Reviewer: Eike Rathke <erack@redhat.com>
Context
Privacy Policy |
Impressum (Legal Info) |
Copyright information: Unless otherwise specified, all text and images
on this website are licensed under the
Creative Commons Attribution-Share Alike 3.0 License.
This does not include the source code of LibreOffice, which is
licensed under the Mozilla Public License (
MPLv2).
"LibreOffice" and "The Document Foundation" are
registered trademarks of their corresponding registered owners or are
in actual use as trademarks in one or more countries. Their respective
logos and icons are also subject to international copyright laws. Use
thereof is explained in our
trademark policy.