Brush up the Viewer example

- Streamline some code
- Fix clang warnings about else after return
- Remove duplicate setting of button text in updateUiState()
- Use member initialization
- Add format string to qCritical

Change-Id: I361687408d9bfabdfefdf0b3bd7fd82f46f9b4cd
Reviewed-by: Rainer Keller <Rainer.Keller@qt.io>

4 files changed, 84 insertions, 73 deletions

diff --git a/examples/opcua/opcuaviewer/certificatedialog.cpp b/examples/opcua/opcuaviewer/certificatedialog.cpp
index dd1b61a..2f98d6f 100644
--- a/examples/opcua/opcuaviewer/certificatedialog.cpp
+++ b/examples/opcua/opcuaviewer/certificatedialog.cpp
@@ -81,7 +81,8 @@ int CertificateDialog::showCertificate(const QString &message, const QByteArray
 void CertificateDialog::saveCertificate()
 {
    const QByteArray digest = m_cert.digest();
-   const QString path = m_trustListDirectory + "/" + digest.toHex() + ".der";
+   const QString path = m_trustListDirectory + QLatin1Char('/')
+       + QLatin1String(digest.toHex()) + QLatin1String(".der");
 
    QFile file(path);
    if (file.open(QIODevice::WriteOnly)) {
diff --git a/examples/opcua/opcuaviewer/mainwindow.cpp b/examples/opcua/opcuaviewer/mainwindow.cpp
index 589cd6d..3f8f1ed 100644
--- a/examples/opcua/opcuaviewer/mainwindow.cpp
+++ b/examples/opcua/opcuaviewer/mainwindow.cpp
@@ -80,29 +80,34 @@ static void messageHandler(QtMsgType type, const QMessageLogContext &context, co
     if (!mainWindowGlobal)
         return;
 
-   QString message = "%1: %2";
-   QString contextStr = " (%1:%2, %3)";
-   QString typeString;
-
-   if (type == QtDebugMsg)
-       typeString = QObject::tr("Debug");
-   else if (type == QtInfoMsg)
-        typeString = QObject::tr("Info");
-   else if (type == QtWarningMsg)
-        typeString = QObject::tr("Warning");
-   else if (type == QtCriticalMsg)
-        typeString = QObject::tr("Critical");
-   else if (type == QtFatalMsg)
-        typeString = QObject::tr("Fatal");
-
-   message = message.arg(typeString).arg(msg);
-   contextStr = contextStr.arg(context.file).arg(context.line).arg(context.function);
-
+   QString message;
    QColor color = Qt::black;
-   if (type == QtFatalMsg || type == QtCriticalMsg)
-       color = Qt::darkRed;
-   else if (type == QtWarningMsg)
+
+   switch (type) {
+   case QtWarningMsg:
+       message = QObject::tr("Warning");
        color = Qt::darkYellow;
+       break;
+   case QtCriticalMsg:
+       message = QObject::tr("Critical");
+       color = Qt::darkRed;
+       break;
+   case QtFatalMsg:
+       message = QObject::tr("Fatal");
+        color = Qt::darkRed;
+       break;
+   case QtInfoMsg:
+       message = QObject::tr("Info");
+       break;
+   case QtDebugMsg:
+       message = QObject::tr("Debug");
+       break;
+   }
+   message += QLatin1String(": ");
+   message += msg;
+
+   const QString contextStr =
+       QStringLiteral(" (%1:%2, %3)").arg(context.file).arg(context.line).arg(context.function);
 
    // Logging messages from backends are sent from different threads and need to be
    // synchronized with the GUI thread.
@@ -127,12 +132,14 @@ MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent)
   , mTreeView(new QTreeView(this))
   , mOpcUaModel(new OpcUaModel(this))
   , mOpcUaProvider(new QOpcUaProvider(this))
-  , mOpcUaClient(nullptr)
-  , mClientConnected(false)
 {
-    int row = 0;
     mainWindowGlobal = this;
 
+    auto centralWidget = new QWidget;
+    auto vbox = new QVBoxLayout(centralWidget);
+    setCentralWidget(centralWidget);
+
+    int row = 0;
     auto grid = new QGridLayout;
     grid->addWidget(new QLabel(tr("Select OPC UA Backend:")), row, 0);
     grid->addWidget(mOpcUaPlugin, row, 1);
@@ -146,16 +153,11 @@ MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent)
     grid->addWidget(mEndpoints, row, 1);
     grid->addWidget(mConnectButton, row, 2);
 
-    auto vbox = new QVBoxLayout;
     vbox->addLayout(grid);
     vbox->addWidget(mTreeView);
     vbox->addWidget(new QLabel(tr("Log:")));
     vbox->addWidget(mLog);
 
-    auto widget = new QWidget;
-    widget->setLayout(vbox);
-    setCentralWidget(widget);
-
     mHost->setText("opc.tcp://localhost:48010");
     mEndpoints->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Preferred);
 
@@ -371,7 +373,7 @@ void MainWindow::updateUiState()
 {
     // allow changing the backend only if it was not already created
     mOpcUaPlugin->setEnabled(mOpcUaClient == nullptr);
-    mConnectButton->setText(mClientConnected?tr("Disconnect"):tr("Connect"));
+    mConnectButton->setText(mClientConnected ? tr("Disconnect") : tr("Connect"));
 
     if (mClientConnected) {
         mHost->setEnabled(false);
@@ -380,7 +382,6 @@ void MainWindow::updateUiState()
         mFindServersButton->setEnabled(false);
         mGetEndpointsButton->setEnabled(false);
         mConnectButton->setEnabled(true);
-        mConnectButton->setText(tr("Disconnect"));
     } else {
         mHost->setEnabled(true);
         mServers->setEnabled(mServers->count() > 0);
@@ -389,7 +390,6 @@ void MainWindow::updateUiState()
         mFindServersButton->setDisabled(mHost->text().isEmpty());
         mGetEndpointsButton->setEnabled(mServers->currentIndex() != -1);
         mConnectButton->setEnabled(mEndpoints->currentIndex() != -1);
-        mConnectButton->setText(tr("Connect"));
     }
 
     if (!mOpcUaClient) {
@@ -424,11 +424,10 @@ bool MainWindow::createPkiPath(const QString &path)
 
     QDir dir;
     const bool ret = dir.mkpath(path);
-    if (ret) {
+    if (ret)
         qDebug() << msg.arg(path).arg("SUCCESS.");
-    } else {
-        qCritical(msg.arg(path).arg("FAILED.").toLocal8Bit());
-    }
+    else
+        qCritical("%s", qPrintable(msg.arg(path).arg("FAILED.")));
 
     return ret;
 }
@@ -456,23 +455,22 @@ bool MainWindow::createPkiFolders()
 
 void MainWindow::showErrorDialog(QOpcUaErrorState *errorState)
 {
-    QString msg;
-    CertificateDialog dlg;
     int result = 0;
 
     const QString statuscode = QOpcUa::statusToString(errorState->errorCode());
 
-    if (errorState->isClientSideError())
-        msg = tr("The client reported: ");
-    else
-        msg = tr("The server reported: ");
+    QString msg = errorState->isClientSideError() ? tr("The client reported: ") : tr("The server reported: ");
 
     switch (errorState->connectionStep()) {
-    case QOpcUaErrorState::ConnectionStep::CertificateValidation:
+    case QOpcUaErrorState::ConnectionStep::Unknown:
+        break;
+    case QOpcUaErrorState::ConnectionStep::CertificateValidation: {
+        CertificateDialog dlg(this);
         msg += tr("Server certificate validation failed with error 0x%1 (%2).\nClick 'Abort' to abort the connect, or 'Ignore' to continue connecting.")
                   .arg(static_cast<ulong>(errorState->errorCode()), 8, 16, QLatin1Char('0')).arg(statuscode);
         result = dlg.showCertificate(msg, m_endpoint.serverCertificate(), m_pkiConfig.trustListDirectory());
         errorState->setIgnoreError(result == 1);
+    }
         break;
     case QOpcUaErrorState::ConnectionStep::OpenSecureChannel:
         msg += tr("OpenSecureChannel failed with error 0x%1 (%2).").arg(errorState->errorCode(), 8, 16, QLatin1Char('0')).arg(statuscode);
diff --git a/examples/opcua/opcuaviewer/mainwindow.h b/examples/opcua/opcuaviewer/mainwindow.h
index cf8b5a6..e19059a 100644
--- a/examples/opcua/opcuaviewer/mainwindow.h
+++ b/examples/opcua/opcuaviewer/mainwindow.h
@@ -104,9 +104,9 @@ private:
     QTreeView *mTreeView;
     OpcUaModel *mOpcUaModel;
     QOpcUaProvider *mOpcUaProvider;
-    QOpcUaClient *mOpcUaClient;
+    QOpcUaClient *mOpcUaClient = nullptr;
     QVector<QOpcUaEndpointDescription> mEndpointList;
-    bool mClientConnected;
+    bool mClientConnected = false;
     QOpcUaApplicationIdentity m_identity;
     QOpcUaPkiConfiguration m_pkiConfig;
     QOpcUaEndpointDescription m_endpoint; // current endpoint used to connect
diff --git a/examples/opcua/opcuaviewer/treeitem.cpp b/examples/opcua/opcuaviewer/treeitem.cpp
index b069189..cbffd53 100644
--- a/examples/opcua/opcuaviewer/treeitem.cpp
+++ b/examples/opcua/opcuaviewer/treeitem.cpp
@@ -129,9 +129,9 @@ int TreeItem::columnCount() const
 
 QVariant TreeItem::data(int column)
 {
-    if (column == 0) {
+    if (column == 0)
         return mNodeBrowseName;
-    } else if (column == 1) {
+    if (column == 1) {
         if (!mAttributesReady)
             return tr("Loading ...");
 
@@ -139,11 +139,13 @@ QVariant TreeItem::data(int column)
         const auto value = mOpcNode->attribute(QOpcUa::NodeAttribute::Value);
 
         return variantToString(value, type);
-    } else if (column == 2) {
+    }
+    if (column == 2) {
         QMetaEnum metaEnum = QMetaEnum::fromType<QOpcUa::NodeClass>();
         QString name = metaEnum.valueToKey((uint)mNodeClass);
         return name + " (" + QString::number((uint)mNodeClass) + ")";
-    } else if (column == 3) {
+    }
+    if (column == 3) {
         if (!mAttributesReady)
             return tr("Loading ...");
 
@@ -153,15 +155,15 @@ QVariant TreeItem::data(int column)
         if (enumEntry == QOpcUa::NodeIds::Namespace0::Unknown)
             return typeId;
         return QOpcUa::namespace0IdName(enumEntry) + " (" + typeId + ")";
-    } else if (column == 4) {
+    }
+    if (column == 4)
         return mNodeId;
-    } else if (column == 5) {
+    if (column == 5)
         return mNodeDisplayName;
-    } else if (column == 6) {
-        if (!mAttributesReady)
-            return tr("Loading ...");
-
-        return mOpcNode->attribute(QOpcUa::NodeAttribute::Description).value<QOpcUaLocalizedText>().text();
+    if (column == 6) {
+        return mAttributesReady
+            ? mOpcNode->attribute(QOpcUa::NodeAttribute::Description).value<QOpcUaLocalizedText>().text()
+            : tr("Loading ...");
     }
     return QVariant();
 }
@@ -294,54 +296,64 @@ QString TreeItem::variantToString(const QVariant &value, const QString &typeNode
     }
     if (typeNodeId == QLatin1String("ns=0;i=2")) // Char
         return QString::number(value.toInt());
-    else if (typeNodeId == QLatin1String("ns=0;i=3")) // SChar
+    if (typeNodeId == QLatin1String("ns=0;i=3")) // SChar
         return QString::number(value.toUInt());
-    else if (typeNodeId == QLatin1String("ns=0;i=4")) // Int16
+    if (typeNodeId == QLatin1String("ns=0;i=4")) // Int16
         return QString::number(value.toInt());
-    else if (typeNodeId == QLatin1String("ns=0;i=5")) // UInt16
+    if (typeNodeId == QLatin1String("ns=0;i=5")) // UInt16
         return QString::number(value.toUInt());
-    else if (value.type() == QVariant::ByteArray)
+    if (value.type() == QVariant::ByteArray)
         return QLatin1String("0x") + value.toByteArray().toHex();
-    else if (value.type() == QVariant::DateTime)
+    if (value.type() == QVariant::DateTime)
         return value.toDateTime().toString(Qt::ISODate);
-    else if (value.canConvert<QOpcUaQualifiedName>()) {
+    if (value.canConvert<QOpcUaQualifiedName>()) {
         const auto name = value.value<QOpcUaQualifiedName>();
         return QStringLiteral("[NamespaceIndex: %1, Name: \"%2\"]").arg(name.namespaceIndex()).arg(name.name());
-    } else if (value.canConvert<QOpcUaLocalizedText>()) {
+    }
+    if (value.canConvert<QOpcUaLocalizedText>()) {
         const auto text = value.value<QOpcUaLocalizedText>();
         return localizedTextToString(text);
-    } else if (value.canConvert<QOpcUaRange>()) {
+    }
+    if (value.canConvert<QOpcUaRange>()) {
         const auto range = value.value<QOpcUaRange>();
         return rangeToString(range);
-    } else if (value.canConvert<QOpcUaComplexNumber>()) {
+    }
+    if (value.canConvert<QOpcUaComplexNumber>()) {
         const auto complex = value.value<QOpcUaComplexNumber>();
         return QStringLiteral("[Real: %1, Imaginary: %2]").arg(complex.real()).arg(complex.imaginary());
-    } else if (value.canConvert<QOpcUaDoubleComplexNumber>()) {
+    }
+    if (value.canConvert<QOpcUaDoubleComplexNumber>()) {
         const auto complex = value.value<QOpcUaDoubleComplexNumber>();
         return QStringLiteral("[Real: %1, Imaginary: %2]").arg(complex.real()).arg(complex.imaginary());
-    } else if (value.canConvert<QOpcUaXValue>()) {
+    }
+    if (value.canConvert<QOpcUaXValue>()) {
         const auto xv = value.value<QOpcUaXValue>();
         return QStringLiteral("[X: %1, Value: %2]").arg(xv.x()).arg(xv.value());
-    } else if (value.canConvert<QOpcUaEUInformation>()) {
+    }
+    if (value.canConvert<QOpcUaEUInformation>()) {
         const auto info = value.value<QOpcUaEUInformation>();
         return euInformationToString(info);
-    } else if (value.canConvert<QOpcUaAxisInformation>()) {
+    }
+    if (value.canConvert<QOpcUaAxisInformation>()) {
         const auto info = value.value<QOpcUaAxisInformation>();
         return QStringLiteral("[EUInformation: %1, EURange: %2, Title: %3 , AxisScaleType: %4, AxisSteps: %5]").arg(
                     euInformationToString(info.engineeringUnits())).arg(rangeToString(info.eURange())).arg(localizedTextToString(info.title())).arg(
                         info.axisScaleType() == QOpcUa::AxisScale::Linear ? "Linear" : (info.axisScaleType() == QOpcUa::AxisScale::Ln) ? "Ln" : "Log").arg(
                         numberArrayToString(info.axisSteps()));
-    } else if (value.canConvert<QOpcUaExpandedNodeId>()) {
+    }
+    if (value.canConvert<QOpcUaExpandedNodeId>()) {
         const auto id = value.value<QOpcUaExpandedNodeId>();
         return QStringLiteral("[NodeId: \"%1\", ServerIndex: \"%2\", NamespaceUri: \"%3\"]").arg(
                     id.nodeId()).arg(id.serverIndex()).arg(id.namespaceUri());
-    } else if (value.canConvert<QOpcUaArgument>()) {
+    }
+    if (value.canConvert<QOpcUaArgument>()) {
         const auto a = value.value<QOpcUaArgument>();
 
         return QStringLiteral("[Name: \"%1\", DataType: \"%2\", ValueRank: \"%3\", ArrayDimensions: %4, Description: %5]").arg(
                     a.name()).arg(a.dataTypeId()).arg(a.valueRank()).arg(numberArrayToString(a.arrayDimensions())).arg(
                     localizedTextToString(a.description()));
-    } else if (value.canConvert<QOpcUaExtensionObject>()) {
+    }
+    if (value.canConvert<QOpcUaExtensionObject>()) {
         const auto obj = value.value<QOpcUaExtensionObject>();
         return QStringLiteral("[TypeId: \"%1\", Encoding: %2, Body: 0x%3]").arg(obj.encodingTypeId()).arg(
                     obj.encoding() == QOpcUaExtensionObject::Encoding::NoBody ?