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

On Friday 11 of January 2013, Noel Grandin wrote:
> 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.

 OUString has a ctor that takes sal_Unicode and creates OUString. Your 
valueOfChar() proposal would add a function that takes sal_Unicode and 
creates OUString => complete duplicate, the ctor can be used instead. I do 
not even see API reasons to have it, it's different from the other valueOf() 
cases in that in the character case there's conceptually actually no 
conversion, so there's no need for consistency.

> > - 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.

 Yes. They would be needed, if the original valueOf() is deprecated/removed.

> > - 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.

 I think I might know why we don't understand each other. There is a code 
problem and you've created new code to solve the problem, and, code-wise, 
your patch is correct. The catch is though that I'm not looking at this from 
the code point of view, but from the API point of view, and it is possible to 
have good code that nevertheless implements API that is not good. IOW, I'm 
not saying that your code is wrong, because I'm thinking one level up and 
saying that the problem you've solved wouldn't even exist at all if the API 
was done better.

 The purpose of libraries is to focus a solution to a problem in a single 
place and provide means to solve the problem. And a good library, among other 
things, solves as much as possible of the problem itself, making it easy for 
the library user. To take an example from elsewhere, cars have a button for 
switching the lights on, it has probably a shining bulb or similar symbol on 
it, and when the driver presses it, it switches the lights on. And that's 
really all the driver needs to know and cares about. The symbol on it doesn't 
show the battery that powers the lights or any other implementation details, 
nor does the button require that you press it with a specific finger. It's 
made as simple as possible and implementation details are hidden. The same 
way, good API should strive to make things as simple as possible and hide 
implementation details.

 Following this, what the developer really wants is to create a string from a 
number. So OUString::fromNumber() seems like the obvious name for it. And it 
should take numbers of any kind, as it doesn't really matter, in the common 
case "give me this number as a string" is conceptually the same whether the 
number is a long or float. So that should be the optimal API for the 
functionality.

 Now some real-world issues may enter into play, e.g. when the number is 
integer, it may be useful to specify the radix, which doesn't make much sense 
with floats; valueOf() has a default argument there, so it can be handled the 
same way. Another thing, since valueOf() is often used when constructing 
strings, OUString::fromNumber() may be considered a bit too long and it may 
be acceptable trade-off to shorten it to OUString::number(). Anything less, 
such as leaking irrelevant implementation details and forcing the developer 
to explicitly specify the underlying type, is settling on sub-optimal API 
that moves part of the implementation burden on the user of the library.

 So, based on this, the best solution to the problem that I can see is 
creating fromNumber() or number() , overloaded for all signed/unsigned 
int/long/long long types and all floats because of the lame language 
ambiguity. The original valueOf() can be wrapped inside #ifndef 
LIBO_INTERNAL_ONLY after LO is moved away from this problematic function to 
keep it only for external backwards compatibility, while LO itself will no 
longer "have" it. So rather than bitting us in small ways every now and then, 
the (possibly smaller in the end, if it wasn't for this discussion) effort is 
spent now, and the effort is not that big (all the duplicates can be only 6 
lines, 2 for doxygen @overload @since, 4 for code forwarding to the overload 
taking the largest type). Win/win. And if this is still not convincing 
enough, then I give up.

-- 
 Lubos Lunak
 l.lunak@suse.cz