From 527c7d2d3cfc6c0b79cbc94107322795e73db34b Mon Sep 17 00:00:00 2001 From: Anthony Bilinski Date: Mon, 7 Oct 2019 21:41:54 -0700 Subject: [PATCH] refactor(history): represent message state as enum class ensures that only one state is set at a time, avoids managing multiple bools --- src/model/chathistory.cpp | 11 ++++++++--- src/model/chatlogitem.h | 4 ++-- src/model/sessionchatlog.cpp | 10 +++++----- src/persistence/history.cpp | 26 +++++++++++++++++++++++--- src/persistence/history.h | 20 ++++++++++++-------- src/widget/form/genericchatform.cpp | 4 ++-- 6 files changed, 52 insertions(+), 23 deletions(-) diff --git a/src/model/chathistory.cpp b/src/model/chathistory.cpp index efadf9bfa..9d937e339 100644 --- a/src/model/chathistory.cpp +++ b/src/model/chathistory.cpp @@ -394,9 +394,14 @@ void ChatHistory::loadHistoryIntoSessionChatLog(ChatLogIdx start) const std::find_if(dispatchedMessageRowIdMap.begin(), dispatchedMessageRowIdMap.end(), [&](RowId dispatchedId) { return dispatchedId == message.id; }); - bool isComplete = dispatchedMessageIt == dispatchedMessageRowIdMap.end(); - auto chatLogMessage = ChatLogMessage{isComplete, processedMessage}; - if (isComplete) { + MessageState messageState; + if (dispatchedMessageIt == dispatchedMessageRowIdMap.end()) { + messageState = MessageState::complete; + } else { + messageState = MessageState::pending; + } + auto chatLogMessage = ChatLogMessage{messageState, processedMessage}; + if (messageState == MessageState::complete) { sessionChatLog.insertCompleteMessageAtIdx(currentIdx, sender, message.dispName, chatLogMessage); } else { diff --git a/src/model/chatlogitem.h b/src/model/chatlogitem.h index 999d1373e..a22a9539c 100644 --- a/src/model/chatlogitem.h +++ b/src/model/chatlogitem.h @@ -23,13 +23,13 @@ #include "src/core/toxfile.h" #include "src/core/toxpk.h" #include "src/model/message.h" +#include "src/persistence/history.h" #include struct ChatLogMessage { - bool isComplete; - bool isBroken; + MessageState state; Message message; }; diff --git a/src/model/sessionchatlog.cpp b/src/model/sessionchatlog.cpp index bc69944d8..f77e5e8b0 100644 --- a/src/model/sessionchatlog.cpp +++ b/src/model/sessionchatlog.cpp @@ -303,7 +303,7 @@ void SessionChatLog::insertCompleteMessageAtIdx(ChatLogIdx idx, const ToxPk& sen item.setDisplayName(senderName); } - assert(message.isComplete == true); + assert(message.state == MessageState::complete); items.emplace(idx, std::move(item)); } @@ -318,7 +318,7 @@ void SessionChatLog::insertIncompleteMessageAtIdx(ChatLogIdx idx, const ToxPk& s item.setDisplayName(senderName); } - assert(message.isComplete == false); + assert(message.state == MessageState::pending); items.emplace(idx, std::move(item)); outgoingMessages.insert(dispatchId, idx); @@ -344,7 +344,7 @@ void SessionChatLog::onMessageReceived(const ToxPk& sender, const Message& messa auto messageIdx = nextIdx++; ChatLogMessage chatLogMessage; - chatLogMessage.isComplete = true; + chatLogMessage.state = MessageState::complete; chatLogMessage.message = message; items.emplace(messageIdx, ChatLogItem(sender, chatLogMessage)); @@ -360,7 +360,7 @@ void SessionChatLog::onMessageSent(DispatchedMessageId id, const Message& messag auto messageIdx = nextIdx++; ChatLogMessage chatLogMessage; - chatLogMessage.isComplete = false; + chatLogMessage.state = MessageState::pending; chatLogMessage.message = message; items.emplace(messageIdx, ChatLogItem(coreIdHandler.getSelfPublicKey(), chatLogMessage)); @@ -390,7 +390,7 @@ void SessionChatLog::onMessageComplete(DispatchedMessageId id) return; } - messageIt->second.getContentAsMessage().isComplete = true; + messageIt->second.getContentAsMessage().state = MessageState::complete; emit this->itemUpdated(messageIt->first); } diff --git a/src/persistence/history.cpp b/src/persistence/history.cpp index 51b98197b..11c0c056b 100644 --- a/src/persistence/history.cpp +++ b/src/persistence/history.cpp @@ -222,6 +222,20 @@ void dbSchemaUpgrade(std::shared_ptr& db) << "->" << SCHEMA_VERSION << ")"; } } + +MessageState getMessageState(bool isPending, bool isBroken) +{ + assert(!(isPending && isBroken)); + MessageState messageState; + if (isPending) { + messageState = MessageState::pending; + } else if (isBroken) { + messageState = MessageState::broken; + } else { + messageState = MessageState::complete; + } + return messageState; +} } // namespace /** @@ -664,8 +678,11 @@ QList History::getMessagesForFriend(const ToxPk& friendPk, auto display_name = QString::fromUtf8(row[4].toByteArray().replace('\0', "")); auto sender_key = row[5].toString(); auto isBroken = !row[13].isNull(); + + MessageState messageState = getMessageState(isPending, isBroken); + if (row[7].isNull()) { - messages += {id, isPending, isBroken, timestamp, friend_key, + messages += {id, messageState, timestamp, friend_key, display_name, sender_key, row[6].toString()}; } else { ToxFile file; @@ -677,7 +694,7 @@ QList History::getMessagesForFriend(const ToxPk& friendPk, file.direction = static_cast(row[11].toLongLong()); file.status = static_cast(row[12].toInt()); messages += - {id, isPending, isBroken, timestamp, friend_key, display_name, sender_key, file}; + {id, messageState, timestamp, friend_key, display_name, sender_key, file}; } }; @@ -711,7 +728,10 @@ QList History::getUndeliveredMessagesForFriend(const ToxPk auto display_name = QString::fromUtf8(row[4].toByteArray().replace('\0', "")); auto sender_key = row[5].toString(); auto isBroken = !row[7].isNull(); - ret += {id, isPending, isBroken, timestamp, friend_key, + + MessageState messageState = getMessageState(isPending, isBroken); + + ret += {id, messageState, timestamp, friend_key, display_name, sender_key, row[6].toString()}; }; diff --git a/src/persistence/history.h b/src/persistence/history.h index 065036b3a..9c5b10cba 100644 --- a/src/persistence/history.h +++ b/src/persistence/history.h @@ -105,33 +105,38 @@ struct FileDbInsertionData }; Q_DECLARE_METATYPE(FileDbInsertionData); +enum class MessageState +{ + complete, + pending, + broken +}; + class History : public QObject, public std::enable_shared_from_this { Q_OBJECT public: struct HistMessage { - HistMessage(RowId id, bool isPending, bool isBroken, QDateTime timestamp, QString chat, QString dispName, + HistMessage(RowId id, MessageState state, QDateTime timestamp, QString chat, QString dispName, QString sender, QString message) : chat{chat} , sender{sender} , dispName{dispName} , timestamp{timestamp} , id{id} - , isPending{isPending} - , isBroken{isBroken} + , state{state} , content(std::move(message)) {} - HistMessage(RowId id, bool isPending, bool isBroken, QDateTime timestamp, QString chat, QString dispName, + HistMessage(RowId id, MessageState state, QDateTime timestamp, QString chat, QString dispName, QString sender, ToxFile file) : chat{chat} , sender{sender} , dispName{dispName} , timestamp{timestamp} , id{id} - , isPending{isPending} - , isBroken{isBroken} + , state{state} , content(std::move(file)) {} @@ -141,8 +146,7 @@ public: QString dispName; QDateTime timestamp; RowId id; - bool isPending; - bool isBroken; + MessageState state; HistMessageContent content; }; diff --git a/src/widget/form/genericchatform.cpp b/src/widget/form/genericchatform.cpp index 2bb1e02be..fc187ad71 100644 --- a/src/widget/form/genericchatform.cpp +++ b/src/widget/form/genericchatform.cpp @@ -179,7 +179,7 @@ ChatMessage::Ptr createMessage(const QString& displayName, bool isSelf, bool col } // Spinner is displayed by passing in an empty date - auto timestamp = chatLogMessage.isComplete ? chatLogMessage.message.timestamp : QDateTime(); + auto timestamp = chatLogMessage.state == MessageState::complete ? chatLogMessage.message.timestamp : QDateTime(); return ChatMessage::createChatMessage(displayName, chatLogMessage.message.content, messageType, isSelf, timestamp, colorizeNames); @@ -190,7 +190,7 @@ void renderMessage(const QString& displayName, bool isSelf, bool colorizeNames, { if (chatMessage) { - if (chatLogMessage.isComplete) { + if (chatLogMessage.state == MessageState::complete) { chatMessage->markAsDelivered(chatLogMessage.message.timestamp); } } else {