QStringList replaceInStrings feature or a bug?

fantaz

Consider the following example:

QStringList mylist;
mylist << "ONE" << "TWO"<<"THREE";
QString after = "dummy";
QString before; // same with QString before=""; 
qDebug()<< mylist;
mylist.replaceInStrings(before, after);
qDebug()<< mylist;

results in an output:

("ONE", "TWO", "THREE")
("dummyOdummyNdummyEdummy", "dummyTdummyWdummyOdummy", "dummyTdummyHdummyRdummyEdummyEdummy")

The replaceInStrings function finds an empty string i.e. the before variable value "", and:

Appends the after string at the end of each string
Prepends the after string before each letter in every string in list

Is this behavior intentional?

jsulm

@fantaz I don't know whether it is intentional. You could check the implementation of replaceInStrings.

fantaz

If I'm not mistaken, this is the actual implementation:

inline QStringList &QStringList::replaceInStrings(const QString &before, const QString &after, Qt::CaseSensitivity cs)
{
    QtPrivate::QStringList_replaceInStrings(this, before, after, cs);
    return *this;
}

void QtPrivate::QStringList_replaceInStrings(QStringList *that, const QString &before,
                                             const QString &after, Qt::CaseSensitivity cs)
{
    for (int i = 0; i < that->size(); ++i)
        (*that)[i].replace(before, after, cs);
}

So, not testing on isEmpty on before QString.
One could either put an if statement in an inline replaceInStrings or QPrivate implementation. I wouldn't know just how much would that slow down the function execution.
I'm not quite sure if this edge case deserves special attention.
On the other hand, the resulting outcome is quite peculiar. I wouldn't expect that the nothing would be replaced with the after string., since it's just that, nothing, an empty string. At least, it would be great if this behavior finds it's way in the documentation.
Am I wrong on this one?

jsulm

@fantaz I think it is up to the developer to make sure the before string isn't empty. This behaviour could be handy actually if you want to put something between characters.

VRonin

You missed a few points in the source, you needed to dig a bit deeper

form QString::replace:

if (d->size == 0) {
        if (blen)
            return *this;
    } else {
        if (cs == Qt::CaseSensitive && before == after && blen == alen)
            return *this;
    }
    if (alen == 0 && blen == 0)
return *this;

so there are checks and your case is explicitly excluded. I'm 100% sure this is an intended feature

fantaz

@jsulm You're right.
IMHO, the best way would be to document this kind of behavior, so that people would know what to expect it they accidentally provide an empty string as a before parameter.

jsulm

@fantaz Agree, it should be documented. You can either provide a patch :-) or create a bug ticket for that.

fantaz

@VRonin Thanks. I missed it!

fantaz

@jsulm Not smart enough to provide the patch, issued a bug ticket instead :-)