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.lu...@suse.cz _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice