From 9dfc807d8f60135976d4ea0ed31022304fad8f4c Mon Sep 17 00:00:00 2001 From: Manuel Nickschas Date: Tue, 26 Feb 2013 23:04:40 +0100 Subject: [PATCH 1/1] 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. --- src/common/protocol.h | 93 +++++++--------------- src/common/protocols/legacy/legacypeer.cpp | 12 +-- src/common/remotepeer.cpp | 2 +- src/common/signalproxy.cpp | 52 ++++++------ 4 files changed, 62 insertions(+), 97 deletions(-) diff --git a/src/common/protocol.h b/src/common/protocol.h index 121f4e45..89625f44 100644 --- 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 ¶ms) - : _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 ¶ms) + : 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 ¶ms) - : _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 ×tamp) : _timestamp(timestamp) {} + inline HeartBeat(const QDateTime ×tamp) : timestamp(timestamp) {} - inline QDateTime timestamp() const { return _timestamp; } - -private: - QDateTime _timestamp; + QDateTime timestamp; }; -class HeartBeatReply +struct HeartBeatReply { -public: - inline HeartBeatReply(const QDateTime ×tamp) : _timestamp(timestamp) {} - - inline QDateTime timestamp() const { return _timestamp; } + inline HeartBeatReply(const QDateTime ×tamp) : timestamp(timestamp) {} -private: - QDateTime _timestamp; + QDateTime timestamp; }; diff --git a/src/common/protocols/legacy/legacypeer.cpp b/src/common/protocols/legacy/legacypeer.cpp index 0f9c61e0..c36baab9 100644 --- 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 3ac55d15..d3752f10 100644 --- 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 1535be71..418fdec8 100644 --- 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()); } -- 2.20.1