I figured out that you could actually pass a slot function without providing it any of its arguments, which of course would cause a Segmentation Fault if the argument was used.
What does that even mean? It's goobledygook. Generally speaking, unless you've managed to build an internally binary-incompatible project (qmake doesn't always protect from that!), Qt won't let you invoke slots with undefined arguments. If the slots have a default argument value, then calling with those arguments missing will substitute default values. But there's never a situation where there's an argument in a slot call whose value will not be set at the very least to a value-initialized default value. I.e. Qt won't blindly invoke your method/function in a binary-incompatible way. That is:
#include <QtCore>
class Class : public QObject {
Q_OBJECT
public:
Q_SIGNAL void mySignal1();
Q_SIGNAL void mySignal2(const QString & = {});
Q_SIGNAL void mySignal3(const QString &);
Q_SIGNAL void mySignal4(QObject *);
Q_SLOT void myMethod1(const QString & = {}) {}
Q_SLOT void myMethod2(const QString &) {}
Q_SLOT void myMethod3(QObject *) {}
};
int main() {
QLoggingCategory::setFilterRules("default.warning=false");
Class c;
bool r;
r = QMetaObject::invokeMethod(&c, "myMethod1");
Q_ASSERT(r); // calls myMethod1, all parameters have values
r = QMetaObject::invokeMethod(&c, "myMethod2");
Q_ASSERT(!r); // doesn't call, value needed for first parameter
r = QMetaObject::invokeMethod(&c, "myMethod2", Q_ARG(QString, "foo"));
Q_ASSERT(r); // calls myMethod2, value is given for first parameter
r = QMetaObject::invokeMethod(&c, "myMethod3");
Q_ASSERT(!r); // doesn't call, value needed for first parameter
r = QObject::connect(&c, SIGNAL(mySignal1()), &c, SLOT(myMethod1()));
Q_ASSERT(r); // succeeds, myMethod1 has a default-value signature
c.mySignal1(); // perfectly defined behavior: mySignal1 gets a default-constructed argument
r = QObject::connect(&c, SIGNAL(mySignal1()), &c, SLOT(myMethod2()));
Q_ASSERT(!r); // fails - no such slot
r = QObject::connect(&c, SIGNAL(mySignal2()), &c, SLOT(myMethod2(QString)));
Q_ASSERT(!r); // fails - no such signal
r = QObject::connect(&c, SIGNAL(mySignal2(QString)), &c, SLOT(myMethod2(QString)));
Q_ASSERT(r);
c.mySignal2(); // mySignal2 has a default value for the argument
r = QObject::connect(&c, SIGNAL(mySignal3(QString)), &c, SLOT(myMethod1(QString)));
Q_ASSERT(r);
c.mySignal3({});
r = QObject::connect(&c, SIGNAL(mySignal4(QObject*)), &c, SLOT(myMethod3(QObject*)));
Q_ASSERT(r);
c.mySignal4(nullptr); // explicit call with a null object, myMethod3 is presumed to accept such
}
#include "main.moc"
In any case, using a null Qt container (i.e. a null string) is not undefined behavior (i.e. doesn't result in a segmentation fault) as long as you're not doing something thoroughly silly like not checking the length of the string you work on when the length plays a role. And it only plays a role if you're trying to use the string in a context where the length is important, e.g. if you're indexing its contents in a way that requires an index within the range of the string. Many methods don't care, i.e. QString::mid
doesn't mind an invalid position - a null string is returned instead.
I am getting into the habit of checking all my arguments to make sure they are not NULL
Your example has nothing to do with checking anything against a null pointer, which is what the C NULL
is!
Your example's test is fairly nonsensical unless somehow m_myString
must be non-empty. I doubt that it is so. A typical Qt setter would look as follows:
void MyClass::setMyString(const QString &input) {
if (m_MyString != input) {
m_MyString = input;
// do whatever is needed when the string has changed, e.g.
// call update() if MyClass is a widget.
emit myStringChanged();
}
}
Note that in Qt, only the default-constructed string is null. Thus, a test for a null string is very specific: it's a test stronger than that for an empty string, and as far as other behavior goes, it doesn't make any difference: a null string and an empty string behave the same, other than the return value from isNull()
.
As a rule of thumb, the null-ness of a string only matters as a discriminant on output. It is a poor-man's workaround for the absence of std::optional
. A null string value is/can be returned in special cases where absence of value needs to be differentiated from an empty string. As far as input goes, everything I know of in Qt treats a null string on input as an empty string, although certainly a null string (or any other null value - i.e. a null pointer or a value with isNull() == true
) propagates to a null variant. But a null string does not result in an invalid variant, for example:
QVariant v;
v = "b";
Q_ASSERT(v.toString() == "b");
v = "";
Q_ASSERT(!v.isNull());
Q_ASSERT(!v.toString().isNull());
Q_ASSERT(v.toString().isEmpty());
QString b;
Q_ASSERT(b.isNull());
v = b;
Q_ASSERT(v.isNull());
Q_ASSERT(v.isValid()); // null but valid
Q_ASSERT(v.toString().isNull());
Q_ASSERT(v.toString().isEmpty());
Q_ASSERT(v.toString() == b);
Q_ASSERT(v != QVariant{}); // because it's valid
QObject o;
o.setProperty("foo", "a");
Q_ASSERT(o.property("foo") == "a");
o.setProperty("foo", QVariant{});
Q_ASSERT(!o.dynamicPropertyNames().contains("foo"));
o.setProperty("foo", QString{}); // not invalid anymore
Q_ASSERT(o.dynamicPropertyNames().contains("foo"));
Q_ASSERT(o.property("foo").isNull());
Thus, if you really for some reason don't want default-constructed (i.e. null) strings coming in, you should assert so. A debug output is insufficient, it should be at least a qWarning
. But an assert would be preferred. Thus:
void MyClass::setMyString(const QString &input) {
Q_ASSERT(!input.isNull());
if (input.isNull()) { // for when QT_NO_DEBUG - i.e. release code
qWarning() << "Error: MyClass::setMyString() invoked with a null parameter";
return;
}
if (m_MyString != input) {
...
}
}
Is this an acceptable style in all or most circumstances?
No. If it's a problem, then a mere qDebug
is not enough. You need an assertion for debug builds, and at least a warning for release builds, if not a release assert. Ideally you'd implement ASSERT_ALWAYS
and use it instead for such cheap checks:
#define ASSERT_ALWAYS(cond) ((cond) ? static_cast<void>(0) : qt_assert(#cond, __FILE__, __LINE__))
Should I be introducing a runtime error, halting the program, instead of simply returning the function?
It depends on the semantics - i.e. meaning of - MyString
. If MyString
makes absolutely no sense when empty, then you should ASSERT_ALWAYS(!input.isEmpty())
(not an isNull
check) and terminate the program. The check is cheap, and the overhead of the check is nothing much compared to the cost of emitting a signal. If MyClass
allows an empty MyString
, then the null check is unnecessary. A null string is an empty string, but an empty string is not necessarily null.
Does this style of coding improve security?
Not really. If MyClass::MyString
cannot ever be empty, then when you attempt to set it to empty, something went horribly wrong. You can't divine how benign of a problem it is, and thus ASSERT_ALWAYS
is the only sane way out: log the problem and die right then and there.
If you want to fail fast like in Erlang, the asserts would throw an assert_exception
of some kind, you'd write exception-safe code, and restart whatever went wrong - but such style requires discipline, and requires extra code to partition the problem into equivalents of Erlang processes that still run on one thread.
Does this style of coding improve stability?
There are only two scenarios where you are improving stability: if you do ASSERT_ALWAYS
- i.e. you prevent continuation upon the first logic error, or if you write fail-fast Erlang-style code where a failing conceptual process is nothing special, and you have means of restarting it. The half-baked "check and do nothing if the argument is wrong" usually has disastrous consequences, as you're ignoring clear logic errors in the user of the class, and assuming that such errors are inconsequential. Yet the class has no knowledge of how inconsequential those errors truly are.
Are there any performance considerations, to the extent that checking for null values may take longer depending on the type?
Again, since you're talking of Qt containers here: your code elsewhere should be already coping with empty containers and not blindly assuming that they have one or more elements - presumably. If the code elsewhere can't deal with it, i.e. it's a class invariant that MyString
has length 1 or more, then of course adding the check has a minuscule performance impact. The cost of the check, for all Qt containers that support null (there aren't many), is a pointer comparison, or two at most.
Beyond C++ and Qt, what other languages benefit from this style?
That's impossible to answer, since you're focused on a null value, which is an implementation detail. The null value is not important. What's important are the semantics of that value in the context it is used. If the null value violates prerequisites for a function (in this case: setMyString
), then of course you should be asserting it, and because it's a cheap check - it should be an ASSERT_ALWAYS
. If the coding style and/or environment supports fail-fast, then you should fail, and let the user code restart the entire failed process (here process means a contained unit of work and has nothing to do with OS processes!).
Are there any significant drawbacks worth considering?
Yes. The fact that you're looking at it from the wrong side - a null QString
is almost never a problem. The semantics of your class, and of the null value in that context, and class invariants, and function prerequisites, are what's important, and you should be looking at the problem in those terms. I have a feeling that whatever was the perceived source of the segmentation faults had zilch to do with null QString
s. Those are, after all, special cases of empty strings, and you can manipulate them just as you could any other empty string. E.g. this is valid:
QString a; // null string is also an empty string
Q_ASSERT(a.isNull());
Q_ASSERT(a.isEmpty());
qDebug() << a;
Q_ASSERT(a.size() == 0);
a += QStringLiteral("b");
Q_ASSERT(a == QStringLiteral("b"));
And just to be completely clear: the following is not a null string. It is not a null string in any shape nor form, because it's not a string to begin with.
QString *s = nullptr;
This is a null pointer-to-string. NOT a STRING. Big difference. And the question contained exactly zero pointers-to-strings, mind you.
nullptr
or notassert
if it would be a programmer error, as part of the standard logic if not.SLOT
macro and not listing arguments in it. Since the macro simply stringifies slot name it is possible to omit argument list and make Qt call the slot without any parameters (which might cause a segfault even withQString
. In that case, the best thing would be to spend some time migrating code to the new pointer-to-method syntax which is type safe.