Make protocol messages structs instead of classes

Encapsulation with private members and accessors is nice, but for something
that is just a POD and will most probably stay that way, it's overkill [1].
So we're turning the classes into structs with direct member access.

Most beneficially, this makes protocol.h much nicer to read.

As a bonus, we introduce a common baseclass that holds the handler() method to
remove duplication.

[1] Even Qt sets a precedent for that, e.g. QPair and many other PODs that are
    implemented as structs with public member access even in the public API.

diff --git a/src/common/protocol.h b/src/common/protocol.h
index 121f4e4..89625f4 100644 (file)
--- a/src/common/protocol.h
+++ b/src/common/protocol.h
@@ -34,105 +34,70 @@ enum Handler {
 
 /*** handled by SignalProxy ***/
 
-class SyncMessage
+struct SignalProxyMessage
 {
-public:
-    inline SyncMessage(const QByteArray &className, const QString &objectName, const QByteArray &slotName, const QVariantList &params)
-    : _className(className), _objectName(objectName), _slotName(slotName), _params(params) {}
-
     inline Handler handler() const { return SignalProxy; }
+};
 
-    inline QByteArray className() const { return _className; }
-    inline QString objectName() const { return _objectName; }
-    inline QByteArray slotName() const { return _slotName; }
 
-    inline QVariantList params() const { return _params; }
+struct SyncMessage : public SignalProxyMessage
+{
+    inline SyncMessage(const QByteArray &className, const QString &objectName, const QByteArray &slotName, const QVariantList &params)
+    : className(className), objectName(objectName), slotName(slotName), params(params) {}
 
-private:
-    QByteArray _className;
-    QString _objectName;
-    QByteArray _slotName;
-    QVariantList _params;
+    QByteArray className;
+    QString objectName;
+    QByteArray slotName;
+    QVariantList params;
 };
 
 
-class RpcCall
+struct RpcCall : public SignalProxyMessage
 {
-public:
     inline RpcCall(const QByteArray &slotName, const QVariantList &params)
-    : _slotName(slotName), _params(params) {}
+    : slotName(slotName), params(params) {}
 
-    inline Handler handler() const { return SignalProxy; }
-
-    inline QByteArray slotName() const { return _slotName; }
-    inline QVariantList params() const { return _params; }
-
-private:
-    QByteArray _slotName;
-    QVariantList _params;
+    QByteArray slotName;
+    QVariantList params;
 };
 
 
-class InitRequest
+struct InitRequest : public SignalProxyMessage
 {
-public:
     inline InitRequest(const QByteArray &className, const QString &objectName)
-    : _className(className), _objectName(objectName) {}
+    : className(className), objectName(objectName) {}
 
-    inline Handler handler() const { return SignalProxy; }
-
-    inline QByteArray className() const { return _className; }
-    inline QString objectName() const { return _objectName; }
-
-private:
-    QByteArray _className;
-    QString _objectName;
+    QByteArray className;
+    QString objectName;
 };
 
 
-class InitData
+struct InitData : public SignalProxyMessage
 {
-public:
     inline InitData(const QByteArray &className, const QString &objectName, const QVariantMap &initData)
-    : _className(className), _objectName(objectName), _initData(initData) {}
-
-    inline Handler handler() const { return SignalProxy; }
+    : className(className), objectName(objectName), initData(initData) {}
 
-    inline QByteArray className() const { return _className; }
-    inline QString objectName() const { return _objectName; }
-
-    inline QVariantMap initData() const { return _initData; }
-
-private:
-    QByteArray _className;
-    QString _objectName;
-    QVariantMap _initData;
+    QByteArray className;
+    QString objectName;
+    QVariantMap initData;
 };
 
 
 /*** handled by RemoteConnection ***/
 
-class HeartBeat
+struct HeartBeat
 {
-public:
-    inline HeartBeat(const QDateTime &timestamp) : _timestamp(timestamp) {}
+    inline HeartBeat(const QDateTime &timestamp) : timestamp(timestamp) {}
 
-    inline QDateTime timestamp() const { return _timestamp; }
-
-private:
-    QDateTime _timestamp;
+    QDateTime timestamp;
 };
 
 
-class HeartBeatReply
+struct HeartBeatReply
 {
-public:
-    inline HeartBeatReply(const QDateTime &timestamp) : _timestamp(timestamp) {}
-
-    inline QDateTime timestamp() const { return _timestamp; }
+    inline HeartBeatReply(const QDateTime &timestamp) : timestamp(timestamp) {}
 
-private:
-    QDateTime _timestamp;
+    QDateTime timestamp;
 };
 
 

diff --git a/src/common/protocols/legacy/legacypeer.cpp b/src/common/protocols/legacy/legacypeer.cpp
index 0f9c61e..c36baab 100644 (file)
--- a/src/common/protocols/legacy/legacypeer.cpp
+++ b/src/common/protocols/legacy/legacypeer.cpp
@@ -232,37 +232,37 @@ void LegacyPeer::handlePackedFunc(const QVariant &packedFunc)
 
 void LegacyPeer::dispatch(const Protocol::SyncMessage &msg)
 {
-    dispatchPackedFunc(QVariantList() << (qint16)Sync << msg.className() << msg.objectName() << msg.slotName() << msg.params());
+    dispatchPackedFunc(QVariantList() << (qint16)Sync << msg.className << msg.objectName << msg.slotName << msg.params);
 }
 
 
 void LegacyPeer::dispatch(const Protocol::RpcCall &msg)
 {
-    dispatchPackedFunc(QVariantList() << (qint16)RpcCall << msg.slotName() << msg.params());
+    dispatchPackedFunc(QVariantList() << (qint16)RpcCall << msg.slotName << msg.params);
 }
 
 
 void LegacyPeer::dispatch(const Protocol::InitRequest &msg)
 {
-    dispatchPackedFunc(QVariantList() << (qint16)InitRequest << msg.className() << msg.objectName());
+    dispatchPackedFunc(QVariantList() << (qint16)InitRequest << msg.className << msg.objectName);
 }
 
 
 void LegacyPeer::dispatch(const Protocol::InitData &msg)
 {
-    dispatchPackedFunc(QVariantList() << (qint16)InitData << msg.className() << msg.objectName() << msg.initData());
+    dispatchPackedFunc(QVariantList() << (qint16)InitData << msg.className << msg.objectName << msg.initData);
 }
 
 
 void LegacyPeer::dispatch(const Protocol::HeartBeat &msg)
 {
-    dispatchPackedFunc(QVariantList() << (qint16)HeartBeat << msg.timestamp().time());
+    dispatchPackedFunc(QVariantList() << (qint16)HeartBeat << msg.timestamp.time());
 }
 
 
 void LegacyPeer::dispatch(const Protocol::HeartBeatReply &msg)
 {
-    dispatchPackedFunc(QVariantList() << (qint16)HeartBeatReply << msg.timestamp().time());
+    dispatchPackedFunc(QVariantList() << (qint16)HeartBeatReply << msg.timestamp.time());
 }
 
 

diff --git a/src/common/remotepeer.cpp b/src/common/remotepeer.cpp
index 3ac55d1..d3752f1 100644 (file)
--- a/src/common/remotepeer.cpp
+++ b/src/common/remotepeer.cpp
@@ -161,7 +161,7 @@ void RemotePeer::close(const QString &reason)
 
 void RemotePeer::handle(const HeartBeat &heartBeat)
 {
-    dispatch(HeartBeatReply(heartBeat.timestamp()));
+    dispatch(HeartBeatReply(heartBeat.timestamp));
 }
 
 

diff --git a/src/common/signalproxy.cpp b/src/common/signalproxy.cpp
index 1535be7..418fdec 100644 (file)
--- a/src/common/signalproxy.cpp
+++ b/src/common/signalproxy.cpp
@@ -504,28 +504,28 @@ void SignalProxy::dispatch(const T &protoMessage)
 
 void SignalProxy::handle(Peer *peer, const SyncMessage &syncMessage)
 {
-    if (!_syncSlave.contains(syncMessage.className()) || !_syncSlave[syncMessage.className()].contains(syncMessage.objectName())) {
-        qWarning() << QString("no registered receiver for sync call: %1::%2 (objectName=\"%3\"). Params are:").arg(syncMessage.className(), syncMessage.slotName(), syncMessage.objectName())
-                   << syncMessage.params();
+    if (!_syncSlave.contains(syncMessage.className) || !_syncSlave[syncMessage.className].contains(syncMessage.objectName)) {
+        qWarning() << QString("no registered receiver for sync call: %1::%2 (objectName=\"%3\"). Params are:").arg(syncMessage.className, syncMessage.slotName, syncMessage.objectName)
+                   << syncMessage.params;
         return;
     }
 
-    SyncableObject *receiver = _syncSlave[syncMessage.className()][syncMessage.objectName()];
+    SyncableObject *receiver = _syncSlave[syncMessage.className][syncMessage.objectName];
     ExtendedMetaObject *eMeta = extendedMetaObject(receiver);
-    if (!eMeta->slotMap().contains(syncMessage.slotName())) {
-        qWarning() << QString("no matching slot for sync call: %1::%2 (objectName=\"%3\"). Params are:").arg(syncMessage.className(), syncMessage.slotName(), syncMessage.objectName())
-                   << syncMessage.params();
+    if (!eMeta->slotMap().contains(syncMessage.slotName)) {
+        qWarning() << QString("no matching slot for sync call: %1::%2 (objectName=\"%3\"). Params are:").arg(syncMessage.className, syncMessage.slotName, syncMessage.objectName)
+                   << syncMessage.params;
         return;
     }
 
-    int slotId = eMeta->slotMap()[syncMessage.slotName()];
+    int slotId = eMeta->slotMap()[syncMessage.slotName];
     if (proxyMode() != eMeta->receiverMode(slotId)) {
         qWarning("SignalProxy::handleSync(): invokeMethod for \"%s\" failed. Wrong ProxyMode!", eMeta->methodName(slotId).constData());
         return;
     }
 
     QVariant returnValue((QVariant::Type)eMeta->returnType(slotId));
-    if (!invokeSlot(receiver, slotId, syncMessage.params(), returnValue)) {
+    if (!invokeSlot(receiver, slotId, syncMessage.params, returnValue)) {
         qWarning("SignalProxy::handleSync(): invokeMethod for \"%s\" failed ", eMeta->methodName(slotId).constData());
         return;
     }
@@ -534,9 +534,9 @@ void SignalProxy::handle(Peer *peer, const SyncMessage &syncMessage)
         int receiverId = eMeta->receiveMap()[slotId];
         QVariantList returnParams;
         if (eMeta->argTypes(receiverId).count() > 1)
-            returnParams << syncMessage.params();
+            returnParams << syncMessage.params;
         returnParams << returnValue;
-        peer->dispatch(SyncMessage(syncMessage.className(), syncMessage.objectName(), eMeta->methodName(receiverId), returnParams));
+        peer->dispatch(SyncMessage(syncMessage.className, syncMessage.objectName, eMeta->methodName(receiverId), returnParams));
     }
 
     // send emit update signal
@@ -546,20 +546,20 @@ void SignalProxy::handle(Peer *peer, const SyncMessage &syncMessage)
 
 void SignalProxy::handle(Peer *peer, const InitRequest &initRequest)
 {
-   if (!_syncSlave.contains(initRequest.className())) {
+   if (!_syncSlave.contains(initRequest.className)) {
         qWarning() << "SignalProxy::handleInitRequest() received initRequest for unregistered Class:"
-                   << initRequest.className();
+                   << initRequest.className;
         return;
     }
 
-    if (!_syncSlave[initRequest.className()].contains(initRequest.objectName())) {
+    if (!_syncSlave[initRequest.className].contains(initRequest.objectName)) {
         qWarning() << "SignalProxy::handleInitRequest() received initRequest for unregistered Object:"
-                   << initRequest.className() << initRequest.objectName();
+                   << initRequest.className << initRequest.objectName;
         return;
     }
 
-    SyncableObject *obj = _syncSlave[initRequest.className()][initRequest.objectName()];
-    peer->dispatch(InitData(initRequest.className(), initRequest.objectName(), initData(obj)));
+    SyncableObject *obj = _syncSlave[initRequest.className][initRequest.objectName];
+    peer->dispatch(InitData(initRequest.className, initRequest.objectName, initData(obj)));
 }
 
 
@@ -567,20 +567,20 @@ void SignalProxy::handle(Peer *peer, const InitData &initData)
 {
     Q_UNUSED(peer)
 
-    if (!_syncSlave.contains(initData.className())) {
+    if (!_syncSlave.contains(initData.className)) {
         qWarning() << "SignalProxy::handleInitData() received initData for unregistered Class:"
-                   << initData.className();
+                   << initData.className;
         return;
     }
 
-    if (!_syncSlave[initData.className()].contains(initData.objectName())) {
+    if (!_syncSlave[initData.className].contains(initData.objectName)) {
         qWarning() << "SignalProxy::handleInitData() received initData for unregistered Object:"
-                   << initData.className() << initData.objectName();
+                   << initData.className << initData.objectName;
         return;
     }
 
-    SyncableObject *obj = _syncSlave[initData.className()][initData.objectName()];
-    setInitData(obj, initData.initData());
+    SyncableObject *obj = _syncSlave[initData.className][initData.objectName];
+    setInitData(obj, initData.initData);
 }
 
 
@@ -590,11 +590,11 @@ void SignalProxy::handle(Peer *peer, const RpcCall &rpcCall)
 
     QObject *receiver;
     int methodId;
-    SlotHash::const_iterator slot = _attachedSlots.constFind(rpcCall.slotName());
-    while (slot != _attachedSlots.constEnd() && slot.key() == rpcCall.slotName()) {
+    SlotHash::const_iterator slot = _attachedSlots.constFind(rpcCall.slotName);
+    while (slot != _attachedSlots.constEnd() && slot.key() == rpcCall.slotName) {
         receiver = (*slot).first;
         methodId = (*slot).second;
-        if (!invokeSlot(receiver, methodId, rpcCall.params())) {
+        if (!invokeSlot(receiver, methodId, rpcCall.params)) {
             ExtendedMetaObject *eMeta = extendedMetaObject(receiver);
             qWarning("SignalProxy::handleSignal(): invokeMethod for \"%s\" failed ", eMeta->methodName(methodId).constData());
         }