From d1978692b8cde1e5d785e31e4d443ae7a52f143f Mon Sep 17 00:00:00 2001 From: Linus Jahn Date: Sat, 11 Jul 2020 22:36:38 +0200 Subject: 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(). --- src/base/QXmppStanza.cpp | 89 +++++++++++--------------------------- src/base/QXmppStanza.h | 6 --- src/base/QXmppStanza_p.h | 44 ++++++++++++++++--- src/base/QXmppStreamManagement.cpp | 7 +-- 4 files changed, 67 insertions(+), 79 deletions(-) (limited to 'src/base') 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 type; + std::optional condition; QString text; QString by; QString redirectionUri; @@ -176,8 +176,6 @@ public: QXmppStanzaErrorPrivate::QXmppStanzaErrorPrivate() : code(0), - type(static_cast(-1)), - condition(static_cast(-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(-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 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 + // 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 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(-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 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(); } -- cgit v1.2.3