Static methods of Transaction could return QStringView instead of allocating a new QString

[ratijas]

All of the following methods do not modify the input string, and neither do they construct a new string from any concatenations that would justify the allocation of a new QString:

src/transaction.h:

    /**
     * Returns the package name from the \p packageID
     */
    static QString packageName(const QString &packageID);

    /**
     * Returns the package version from the \p packageID
     */
    static QString packageVersion(const QString &packageID);

    /**
     * Returns the package arch from the \p packageID
     */
    static QString packageArch(const QString &packageID);

    /**
     * Returns the package data from the \p packageID
     */
    static QString packageData(const QString &packageID);

In KDE/Plasma Discover there is a copy-pasta of packageName method for the sake of performance improvement. Here's a copy of that comment of top:

// Copy of Transaction::packageName that doesn't create a copy but just pass a reference
// It's an optimisation as there's a bunch of allocations that happen from packageName
// Having packageName return a QStringRef or a QStringView would fix this issue.
// TODO Qt 6: Have packageName and similars return a QStringView

Changing it in packagekit-qt now would be an ABI brakage, and you can't overload by return types in C++. But it should at least still be mostly API-compatible, as QStringView implicitly converts to QString when needed.

Alternatively, we could overloaded counterparts for all those methods which both take and return a string view. That would require explicit porting in apps, if they want to take advantage of performance gains.

[aleixpol]

It would break ABI and API. Even as KDE, so near to 6.0, this is too late.

I would suggest just creating the overloads and take the hit on having a slightly cleaner API. We can add a QStringView packageNameView() const method and move on from this issue.

I'm pretty sure we already discussed this with QStringRef, which is almost the same as QStringView, alas here we are again.

Further analysis on whether a PackageKit::PackageId class that caches the positions of the ; could make sense, this can be done later down the road though. It would also bring some type safety which always helps.