On 01/10/2013 02:55 PM, Lubos Lunak wrote:
On Thursday 10 of January 2013, Noel Grandin wrote:
https://gerrit.libreoffice.org/#/c/1625/

  Can we please keep the discussion still here? Gerrit may be fine for pointing
out technical details in the code, but it's not very suitable for discussions
about anything beyond that.

- There's no need for valueOfChar(). There is already OUString ctor from
sal_Unicode, so the valueOf() overload for it is just making an obvious thing
complicated. Code using it can be converted to use the ctor instead.

- It's a question if we really need 'OUString::valueOfBool( foo )' instead of
simply 'foo ? OUString( "true" ) : OUString( "false" )' (such a pity the
string literals handling doesn't allow "foo ? "true" : "false"' ). I wonder
how many places in the code really need to convert a boolean to the hardcoded
english string representation.

- When more or less deprecating valueOf() this way, it has also float
overloads, so something should be created for those too.

- I'm still not sold on the naming, OUString::valueInt() doesn't say much and
OUString::valueOfInt() feels cryptic. Can we please use something obvious
that doesn't need decyphering, such as OUString::number() or
OUString::fromInt() (as much as I still don't like the idea of harcoding the
irrelevant type information in the name)?

Btw, the name "valueOf" and the idea of overloading it for all conceivable (sal) types comes from the -- somewhat misguided -- attempt at modelling rtll::OUString after java.lang.String.

The "fromInt" etc. names look OK to me, too.

Re dropping valueOfBool, I'm not sure how many places actually use it; "for i in solver/*/lib/*.so*; do nm "$i" -D --undef | grep -F _valueOfBoolean; done" does indicate its not completely unused. Also, we have rtl_ustr_valueOfBoolean around anyway.

Stephan
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Reply via email to