replacing OUString::valueOf(static_cast<sal_Int32>) ??

Noel Grandin noel at peralex.com
Thu Jan 10 22:46:40 PST 2013


On 2013-01-10 20:03, Lubos Lunak wrote:
> That is not what I meant. What I wrote in my previous mail is that if 
> these valueOf() issues are to be fixed, it's better to fix it 
> completely rather than just slightly reduce the problem. And for that 
> it would be better to remove the original valueOf() completely, and 
> that results in all the follow-up issues that I raised. 
Ah, I see.

So you're saying that you'd rather we remove the original valueOf() 
altogether.
That seems like it would have some backwards compatibility issues, but I 
suppose we could mark them as deprecated for a time before actually 
removing them.

In that case, let's deal with your issues one at a time:

On 2013-01-10 15:55, Lubos Lunak wrote:
> - 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.
I've already dealt with why we can't use the constructor.
    git grep "String::valueOf.*Unicode"
says we do in fact appear to need such a method.

> - 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.
I have no idea how to count call sites automatically.
But at least 10 places need to use a cast to access the method, so there 
is definitely code using it.

> - When more or less deprecating valueOf() this way, it has also float
> overloads, so something should be created for those too.
Fair enough. We don't have many of them, but for completeness sake, it 
makes sense.


> - 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 , 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)?

I think valueOf() is fine, but then I do a lot of programming in Java :-)
But whatever, if the C++ people prefer another name, then so be it.


Disclaimer: http://www.peralex.com/disclaimer.html




More information about the LibreOffice mailing list