QXmppStanza::Error: Use std::optional<> internally

This makes the variables for the error type and condition an
std::optional<> as this makes the meaning clearer than hidden -1 values
created by dubious casts.

For now, the API is not changed, because we can't replace the getter
easily. We could do something like type() and optionalType().

4 files changed, 67 insertions, 79 deletions

diff --git a/src/base/QXmppStanza.cpp b/src/base/QXmppStanza.cpp
index 859f0977..76b9f5f8 100644
--- a/src/base/QXmppStanza.cpp
+++ b/src/base/QXmppStanza.cpp
@@ -162,8 +162,8 @@ public:
     QXmppStanzaErrorPrivate();
 
     int code;
-    QXmppStanza::Error::Type type;
-    QXmppStanza::Error::Condition condition;
+    std::optional<QXmppStanza::Error::Type> type;
+    std::optional<QXmppStanza::Error::Condition> condition;
     QString text;
     QString by;
     QString redirectionUri;
@@ -176,8 +176,6 @@ public:
 
 QXmppStanzaErrorPrivate::QXmppStanzaErrorPrivate()
     : code(0),
-      type(static_cast<QXmppStanza::Error::Type>(-1)),
-      condition(static_cast<QXmppStanza::Error::Condition>(-1)),
       fileTooLarge(false)
 {
 }
@@ -212,8 +210,8 @@ QXmppStanza::Error::Error(const QString &type, const QString &cond,
     : d(new QXmppStanzaErrorPrivate)
 {
     d->text = text;
-    setTypeFromStr(type);
-    setConditionFromStr(cond);
+    d->type = typeFromString(type);
+    d->condition = conditionFromString(cond);
 }
 
 /// Default destructor
@@ -262,7 +260,7 @@ void QXmppStanza::Error::setCode(int code)
 ///
 QXmppStanza::Error::Condition QXmppStanza::Error::condition() const
 {
-    return d->condition;
+    return d->condition.value_or(QXmppStanza::Error::Condition(-1));
 }
 
 ///
@@ -273,6 +271,10 @@ QXmppStanza::Error::Condition QXmppStanza::Error::condition() const
 ///
 void QXmppStanza::Error::setCondition(QXmppStanza::Error::Condition cond)
 {
+    if (int(cond) < 0) {
+        d->condition = std::nullopt;
+        return;
+    }
     d->condition = cond;
 }
 
@@ -281,7 +283,7 @@ void QXmppStanza::Error::setCondition(QXmppStanza::Error::Condition cond)
 ///
 QXmppStanza::Error::Type QXmppStanza::Error::type() const
 {
-    return d->type;
+    return d->type.value_or(QXmppStanza::Error::Type(-1));
 }
 
 ///
@@ -315,6 +317,10 @@ void QXmppStanza::Error::setBy(const QString &by)
 ///
 void QXmppStanza::Error::setType(QXmppStanza::Error::Type type)
 {
+    if (int(type) < 0) {
+        d->type = std::nullopt;
+        return;
+    }
     d->type = type;
 }
 
@@ -403,63 +409,19 @@ void QXmppStanza::Error::setRetryDate(const QDateTime &retryDate)
 }
 
 /// \cond
-QString QXmppStanza::Error::getTypeStr() const
-{
-    switch (d->type) {
-    case Cancel:
-        return QStringLiteral("cancel");
-    case Continue:
-        return QStringLiteral("continue");
-    case Modify:
-        return QStringLiteral("modify");
-    case Auth:
-        return QStringLiteral("auth");
-    case Wait:
-        return QStringLiteral("wait");
-    default:
-        return {};
-    }
-}
-
-QString QXmppStanza::Error::getConditionStr() const
-{
-    return strFromCondition(d->condition);
-}
-
-void QXmppStanza::Error::setTypeFromStr(const QString &type)
-{
-    if (type == QStringLiteral("cancel"))
-        setType(Cancel);
-    else if (type == QStringLiteral("continue"))
-        setType(Continue);
-    else if (type == QStringLiteral("modify"))
-        setType(Modify);
-    else if (type == QStringLiteral("auth"))
-        setType(Auth);
-    else if (type == QStringLiteral("wait"))
-        setType(Wait);
-    else
-        setType(static_cast<QXmppStanza::Error::Type>(-1));
-}
-
-void QXmppStanza::Error::setConditionFromStr(const QString &type)
-{
-    setCondition(conditionFromStr(type));
-}
-
 void QXmppStanza::Error::parse(const QDomElement &errorElement)
 {
-    setCode(errorElement.attribute(QStringLiteral("code")).toInt());
-    setTypeFromStr(errorElement.attribute(QStringLiteral("type")));
-    setBy(errorElement.attribute(QStringLiteral("by")));
+    d->code = errorElement.attribute(QStringLiteral("code")).toInt();
+    d->type = typeFromString(errorElement.attribute(QStringLiteral("type")));
+    d->by = errorElement.attribute(QStringLiteral("by"));
 
     QDomElement element = errorElement.firstChildElement();
     while (!element.isNull()) {
         if (element.namespaceURI() == ns_stanza) {
             if (element.tagName() == QStringLiteral("text")) {
-                setText(element.text());
+                d->text = element.text();
             } else {
-                setConditionFromStr(element.tagName());
+                d->condition = conditionFromString(element.tagName());
 
                 // redirection URI
                 if (d->condition == Gone || d->condition == Redirect) {
@@ -490,21 +452,20 @@ void QXmppStanza::Error::parse(const QDomElement &errorElement)
 
 void QXmppStanza::Error::toXml(QXmlStreamWriter *writer) const
 {
-    QString cond = getConditionStr();
-    QString type = getTypeStr();
-
-    if (cond.isEmpty() && type.isEmpty())
+    if (!d->condition && !d->type)
         return;
 
     writer->writeStartElement(QStringLiteral("error"));
     helperToXmlAddAttribute(writer, QStringLiteral("by"), d->by);
-    helperToXmlAddAttribute(writer, QStringLiteral("type"), type);
+    if (d->type) {
+        writer->writeAttribute(QStringLiteral("type"), typeToString(*d->type));
+    }
 
     if (d->code > 0)
         helperToXmlAddAttribute(writer, QStringLiteral("code"), QString::number(d->code));
 
-    if (!cond.isEmpty()) {
-        writer->writeStartElement(cond);
+    if (d->condition) {
+        writer->writeStartElement(conditionToString(*d->condition));
         writer->writeDefaultNamespace(ns_stanza);
 
         // redirection URI
diff --git a/src/base/QXmppStanza.h b/src/base/QXmppStanza.h
index caf6d6df..03a0680b 100644
--- a/src/base/QXmppStanza.h
+++ b/src/base/QXmppStanza.h
@@ -190,12 +190,6 @@ public:
         /// \endcond
 
     private:
-        QString getConditionStr() const;
-        void setConditionFromStr(const QString &cond);
-
-        QString getTypeStr() const;
-        void setTypeFromStr(const QString &type);
-
         QSharedDataPointer<QXmppStanzaErrorPrivate> d;
     };
 
diff --git a/src/base/QXmppStanza_p.h b/src/base/QXmppStanza_p.h
index 03875639..89fa3209 100644
--- a/src/base/QXmppStanza_p.h
+++ b/src/base/QXmppStanza_p.h
@@ -26,6 +26,8 @@
 
 #include "QXmppStanza.h"
 
+#include <optional>
+
 //  W A R N I N G
 //  -------------
 //
@@ -38,7 +40,7 @@
 // We mean it.
 //
 
-static QString strFromCondition(const QXmppStanza::Error::Condition& condition)
+static QString conditionToString(QXmppStanza::Error::Condition condition)
 {
     switch (condition) {
     case QXmppStanza::Error::BadRequest:
@@ -87,12 +89,11 @@ static QString strFromCondition(const QXmppStanza::Error::Condition& condition)
         return "undefined-condition";
     case QXmppStanza::Error::UnexpectedRequest:
         return "unexpected-request";
-    default:
-        return "";
     }
+    return {};
 }
 
-static QXmppStanza::Error::Condition conditionFromStr(const QString& string)
+static std::optional<QXmppStanza::Error::Condition> conditionFromString(const QString &string)
 {
     if (string == "bad-request")
         return QXmppStanza::Error::BadRequest;
@@ -140,8 +141,39 @@ static QXmppStanza::Error::Condition conditionFromStr(const QString& string)
         return QXmppStanza::Error::UndefinedCondition;
     else if (string == "unexpected-request")
         return QXmppStanza::Error::UnexpectedRequest;
-    else
-        return static_cast<QXmppStanza::Error::Condition>(-1);
+    return std::nullopt;
+}
+
+static QString typeToString(QXmppStanza::Error::Type type)
+{
+    switch (type) {
+    case QXmppStanza::Error::Cancel:
+        return QStringLiteral("cancel");
+    case QXmppStanza::Error::Continue:
+        return QStringLiteral("continue");
+    case QXmppStanza::Error::Modify:
+        return QStringLiteral("modify");
+    case QXmppStanza::Error::Auth:
+        return QStringLiteral("auth");
+    case QXmppStanza::Error::Wait:
+        return QStringLiteral("wait");
+    }
+    return {};
+}
+
+static std::optional<QXmppStanza::Error::Type> typeFromString(const QString &string)
+{
+    if (string == QStringLiteral("cancel"))
+        return QXmppStanza::Error::Cancel;
+    else if (string == QStringLiteral("continue"))
+        return QXmppStanza::Error::Continue;
+    else if (string == QStringLiteral("modify"))
+        return QXmppStanza::Error::Modify;
+    else if (string == QStringLiteral("auth"))
+        return QXmppStanza::Error::Auth;
+    else if (string == QStringLiteral("wait"))
+        return QXmppStanza::Error::Wait;
+    return std::nullopt;
 }
 
 #endif
diff --git a/src/base/QXmppStreamManagement.cpp b/src/base/QXmppStreamManagement.cpp
index 5f1e735b..45f0b94c 100644
--- a/src/base/QXmppStreamManagement.cpp
+++ b/src/base/QXmppStreamManagement.cpp
@@ -262,17 +262,18 @@ void QXmppStreamManagementFailed::parse(const QDomElement &element)
 {
     QDomElement childElement = element.firstChildElement();
     if (!childElement.isNull() && childElement.namespaceURI() == ns_stanza) {
-        m_error = conditionFromStr(childElement.tagName());
+        m_error = conditionFromString(childElement.tagName()).value_or(QXmppStanza::Error::Condition(-1));
     }
 }
 
 void QXmppStreamManagementFailed::toXml(QXmlStreamWriter *writer) const
 {
-    QString errorString = strFromCondition(m_error);
+    QString errorString = conditionToString(m_error);
 
     writer->writeStartElement(QStringLiteral("failed"));
     writer->writeDefaultNamespace(ns_stream_management);
-    writer->writeStartElement(errorString, ns_stanza);
+    writer->writeStartElement(errorString);
+    writer->writeDefaultNamespace(ns_stanza);
     writer->writeEndElement();
     writer->writeEndElement();
 }