On Wed, May 31, 2023 at 07:17:21AM +0000, Marc Mutz via Development wrote: > On 31.05.23 09:07, Sami Varanka via Development wrote: > > What is the recommended way for functions to take strings? Our QtGraphs > > API takes in const QString & but is recommended way nowadays take in > > QAnyStringView? > > I doubt there's an accepted project-wide standard, yet, but as a rule of > thumb that everyone might be able to agree on: If the function doesn't > store the string as-is (=parses or pre-processes it), take by > QAnyStringView, otherwise continue to take by QString cref. > > For very important APIs, there's the option to overload on > > - `const QString &` and/or `QString &&` > - QAnyStringView > > In this case, the QString overloads have to be marked Q_WEAK_OVERLOAD to > avoid ambiguous calls with certain argument types (`const char*`, > `QByteArray`, etc).
Close, but I'd actually further qualify the precondition "If the function doesn't store the string as-is" and possibly quantify the whole thing to "If the function doesn't *necessarily* store the string as-is (=parses or pre-processes it), take by QAnyStringView, otherwise continue to take by QString cref. For *a rare, but* very important APIs, there's the option to overload on - `const QString &` and/or `QString &&` - QAnyStringView" My reason for the "*necessarily*" is to not leak an incidental implementation detail of a given implementation into the long-term API. A "current" implementation for a certain function might get away with not storing a copy, a possibly following improved version might actually _need_ a copy. There should be no restriction on future implementations of such functions that might only be possible by storing a copy. For the "rare": I'd like to re-iterate that introducing the /first/ overload for /anything/ is source-incompatible. I was reminded /again/ of that after updating to current Qt dev and finding out the hard way that some code that compiled for ages (using "&QVariant::fromValue<int>") doesn't compile anymore due to some change that introduced a generic r-value overload for it. I will forgo any public pondering over (im-)possible setups where r-values for int parameter have /any/ benefit and simply hope that this will fixed in the actual 6.7 release, my point /here/ is that overloads should not be added lightheartedly. Ever. In the context of the original poster ("Qt Graph API") this means that since any string-ish thing passed to that API will likely to be passed through several more layers of API and finally be subject to some kind of optimized delayed(!) rendering of the string, a generic benefit of having a QAnyStringView is rather unlikely. /Optimal/ in that context is some ownership-transfering mechanism taking input in the finally used encoding, second to that is anything that created at least one deep copy. In that situation a non-overloaded 'const QString &ref' parameter is simply "good enough". Andre' -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development