Change in core[master]: Replaced deprecated tools/String with OUString in ScAddInCol

[Gerrit]

>From Eike Rathke <erack at 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 at gmail.com>
Gerrit-Reviewer: Eike Rathke <erack at redhat.com>