chore: apply some coding-style suggestions from SonarCloud

Patric Stout
1 parent 5454f1c4
Showing 7 changed files with 82 additions and 72 deletions
include/TrueMQTT.h
src/Client.cpp
src/ClientImpl.h
src/Connection.cpp
src/Connection.h
src/Log.h
src/Packet.cpp
@@ -132,7 +132,7 @@ namespace TrueMQTT
          * @note This library doesn't contain a logger, so you need to provide one.
          * If this method is not called, no logging will be done.
          */
-        void setLogger(LogLevel log_level, std::function<void(LogLevel, std::string)> logger);
+        void setLogger(LogLevel log_level, const std::function<void(LogLevel, std::string)> &logger) const;
         /**
          * @brief Set the last will message on the connection.
@@ -143,14 +143,14 @@ namespace TrueMQTT
          *
          * @note Cannot be called after \ref connect.
          */
-        void setLastWill(const std::string &topic, const std::string &payload, bool retain);
+        void setLastWill(const std::string &topic, const std::string &payload, bool retain) const;
         /**
          * @brief Set the error callback, called when any error occurs.
          *
          * @param callback The callback to call when an error occurs.
          */
-        void setErrorCallback(std::function<void(Error, std::string)> callback);
+        void setErrorCallback(const std::function<void(Error, std::string)> &callback) const;
         /**
          * @brief Set the publish queue to use.
@@ -160,7 +160,7 @@ namespace TrueMQTT
          *
          * @note Cannot be called after \ref connect.
          */
-        void setPublishQueue(PublishQueueType queue_type, size_t size);
+        void setPublishQueue(PublishQueueType queue_type, size_t size) const;
         /**
          * @brief Connect to the broker.
@@ -174,7 +174,7 @@ namespace TrueMQTT
          *
          * @note Calling connect twice has no effect.
          */
-        void connect();
+        void connect() const;
         /**
          * @brief Disconnect from the broker.
@@ -187,7 +187,7 @@ namespace TrueMQTT
          * moment the connection to the broker is established, and there are messages in the
          * publish queue and/or subscriptions.
          */
-        void disconnect();
+        void disconnect() const;
         /**
          * @brief Publish a payload on a topic.
@@ -210,7 +210,7 @@ namespace TrueMQTT
          * moment the connection to the broker is established, and there are messages in the
          * publish queue and/or subscriptions.
          */
-        void publish(const std::string &topic, const std::string &payload, bool retain);
+        void publish(const std::string &topic, const std::string &payload, bool retain) const;
         /**
          * @brief Subscribe to a topic, and call the callback function when a message arrives.
@@ -242,7 +242,7 @@ namespace TrueMQTT
          * moment the connection to the broker is established, and there are messages in the
          * publish queue and/or subscriptions.
          */
-        void subscribe(const std::string &topic, std::function<void(std::string, std::string)> callback);
+        void subscribe(const std::string &topic, const std::function<void(std::string, std::string)> &callback) const;
         /**
          * @brief Unsubscribe from a topic.
@@ -258,7 +258,7 @@ namespace TrueMQTT
          * moment the connection to the broker is established, and there are messages in the
          * publish queue and/or subscriptions.
          */
-        void unsubscribe(const std::string &topic);
+        void unsubscribe(const std::string &topic) const;
     private:
         // Private implementation
@@ -28,7 +28,7 @@ Client::~Client()
     this->disconnect();
 }
-void Client::setLogger(Client::LogLevel log_level, std::function<void(Client::LogLevel, std::string)> logger)
+void Client::setLogger(Client::LogLevel log_level, const std::function<void(Client::LogLevel, std::string)> &logger) const
 {
     LOG_TRACE(this->m_impl, "Setting logger to log level " + std::to_string(log_level));
@@ -38,7 +38,7 @@ void Client::setLogger(Client::LogLevel log_level, std::function&lt;void(Client::Lo
     LOG_DEBUG(this->m_impl, "Log level now on " + std::to_string(this->m_impl->log_level));
 }
-void Client::setLastWill(const std::string &topic, const std::string &payload, bool retain)
+void Client::setLastWill(const std::string &topic, const std::string &payload, bool retain) const
 {
     if (this->m_impl->state != Client::Impl::State::DISCONNECTED)
     {
@@ -53,14 +53,14 @@ void Client::setLastWill(const std::string &amp;topic, const std::string &amp;payload, b
     this->m_impl->last_will_retain = retain;
 }
-void Client::setErrorCallback(std::function<void(Error, std::string)> callback)
+void Client::setErrorCallback(const std::function<void(Error, std::string)> &callback) const
 {
     LOG_TRACE(this->m_impl, "Setting error callback");
     this->m_impl->error_callback = callback;
 }
-void Client::setPublishQueue(Client::PublishQueueType queue_type, size_t size)
+void Client::setPublishQueue(Client::PublishQueueType queue_type, size_t size) const
 {
     if (this->m_impl->state != Client::Impl::State::DISCONNECTED)
     {
@@ -74,7 +74,7 @@ void Client::setPublishQueue(Client::PublishQueueType queue_type, size_t size)
     this->m_impl->publish_queue_size = size;
 }
-void Client::connect()
+void Client::connect() const
 {
     std::scoped_lock lock(this->m_impl->state_mutex);
@@ -89,7 +89,7 @@ void Client::connect()
     this->m_impl->connect();
 }
-void Client::disconnect()
+void Client::disconnect() const
 {
     std::scoped_lock lock(this->m_impl->state_mutex);
@@ -105,7 +105,7 @@ void Client::disconnect()
     this->m_impl->disconnect();
 }
-void Client::publish(const std::string &topic, const std::string &payload, bool retain)
+void Client::publish(const std::string &topic, const std::string &payload, bool retain) const
 {
     std::scoped_lock lock(this->m_impl->state_mutex);
@@ -125,7 +125,7 @@ void Client::publish(const std::string &amp;topic, const std::string &amp;payload, bool 
     }
 }
-void Client::subscribe(const std::string &topic, std::function<void(std::string, std::string)> callback)
+void Client::subscribe(const std::string &topic, const std::function<void(std::string, std::string)> &callback) const
 {
     std::scoped_lock lock(this->m_impl->state_mutex);
@@ -143,10 +143,10 @@ void Client::subscribe(const std::string &amp;topic, std::function&lt;void(std::string,
     std::getline(stopic, part, '/');
     // Find the root node, and walk down till we find the leaf node.
-    Client::Impl::SubscriptionPart *subscriptions = &this->m_impl->subscriptions.try_emplace(part, Client::Impl::SubscriptionPart()).first->second;
+    Client::Impl::SubscriptionPart *subscriptions = &this->m_impl->subscriptions.try_emplace(part).first->second;
     while (std::getline(stopic, part, '/'))
     {
-        subscriptions = &subscriptions->children.try_emplace(part, Client::Impl::SubscriptionPart()).first->second;
+        subscriptions = &subscriptions->children.try_emplace(part).first->second;
     }
     // Add the callback to the leaf node.
     subscriptions->callbacks.push_back(callback);
@@ -158,7 +158,7 @@ void Client::subscribe(const std::string &amp;topic, std::function&lt;void(std::string,
     }
 }
-void Client::unsubscribe(const std::string &topic)
+void Client::unsubscribe(const std::string &topic) const
 {
     std::scoped_lock lock(this->m_impl->state_mutex);
@@ -178,11 +178,11 @@ void Client::unsubscribe(const std::string &amp;topic)
     // Find the root node, and walk down till we find the leaf node.
     std::vector<std::tuple<std::string, Client::Impl::SubscriptionPart *>> reverse;
     Client::Impl::SubscriptionPart *subscriptions = &this->m_impl->subscriptions[part];
-    reverse.push_back({part, subscriptions});
+    reverse.emplace_back(part, subscriptions);
     while (std::getline(stopic, part, '/'))
     {
         subscriptions = &subscriptions->children[part];
-        reverse.push_back({part, subscriptions});
+        reverse.emplace_back(part, subscriptions);
     }
     // Clear the callbacks in the leaf node.
     subscriptions->callbacks.clear();
@@ -240,9 +240,9 @@ void Client::Impl::connectionStateChange(bool connected)
             this->sendSubscribe(subscription);
         }
         // Flush the publish queue.
-        for (auto &message : this->publish_queue)
+        for (const auto &[topic, payload, retain] : this->publish_queue)
         {
-            this->sendPublish(std::get<0>(message), std::get<1>(message), std::get<2>(message));
+            this->sendPublish(topic, payload, retain);
         }
         this->publish_queue.clear();
     }
@@ -283,13 +283,13 @@ void Client::Impl::toPublishQueue(const std::string &amp;topic, const std::string &amp;p
     }
     LOG_TRACE(this, "Adding message to publish queue");
-    this->publish_queue.push_back({topic, payload, retain});
+    this->publish_queue.emplace_back(topic, payload, retain);
 }
 void Client::Impl::findSubscriptionMatch(std::vector<std::function<void(std::string, std::string)>> &matching_callbacks, const std::map<std::string, Client::Impl::SubscriptionPart> &subscriptions, std::deque<std::string> &parts)
 {
     // If we reached the end of the topic, do nothing anymore.
-    if (parts.size() == 0)
+    if (parts.empty())
     {
         return;
     }
@@ -357,7 +357,7 @@ void Client::Impl::messageReceived(std::string topic, std::string payload)
     }
     else
     {
-        for (auto &callback : matching_callbacks)
+        for (const auto &callback : matching_callbacks)
         {
             callback(topic, payload);
         }
@@ -68,14 +68,14 @@ public:
     int connection_backoff_max; ///< Maximum time between backoff attempts in seconds.
     int keep_alive_interval;    ///< Interval in seconds between keep-alive messages.
-    Client::LogLevel log_level = Client::LogLevel::NONE;                                              ///< The log level to use.
-    std::function<void(Client::LogLevel, std::string)> logger = [](Client::LogLevel, std::string) {}; ///< Logger callback.
+    Client::LogLevel log_level = Client::LogLevel::NONE;                                                                      ///< The log level to use.
+    std::function<void(Client::LogLevel, std::string)> logger = std::move([](Client::LogLevel, std::string) { /* empty */ }); ///< Logger callback.
     std::string last_will_topic = "";   ///< Topic to publish the last will message to.
     std::string last_will_payload = ""; ///< Payload of the last will message.
     bool last_will_retain = false;      ///< Whether to retain the last will message.
-    std::function<void(Error, std::string)> error_callback = [](Error, std::string) {}; ///< Error callback.
+    std::function<void(Error, std::string)> error_callback = std::move([](Error, std::string) { /* empty */ }); ///< Error callback.
     Client::PublishQueueType publish_queue_type = Client::PublishQueueType::DROP; ///< The type of queue to use for the publish queue.
     size_t publish_queue_size = -1;                                               ///< Size of the publish queue.
@@ -17,10 +17,10 @@
 #include <vector>
 Connection::Connection(TrueMQTT::Client::LogLevel log_level,
-                       const std::function<void(TrueMQTT::Client::LogLevel, std::string)> logger,
-                       const std::function<void(TrueMQTT::Client::Error, std::string)> error_callback,
-                       const std::function<void(std::string, std::string)> publish_callback,
-                       const std::function<void(bool)> connection_change_callback,
+                       const std::function<void(TrueMQTT::Client::LogLevel, std::string)> &logger,
+                       const std::function<void(TrueMQTT::Client::Error, std::string)> &error_callback,
+                       const std::function<void(std::string, std::string)> &publish_callback,
+                       const std::function<void(bool)> &connection_change_callback,
                        const std::string &host,
                        int port)
     : log_level(log_level),
@@ -46,17 +46,17 @@ Connection::~Connection()
     // freeaddrinfo() is one of those functions that doesn't take kind to NULL pointers
     // on some platforms.
-    if (this->m_host_resolved != NULL)
+    if (this->m_host_resolved != nullptr)
     {
         freeaddrinfo(this->m_host_resolved);
-        this->m_host_resolved = NULL;
+        this->m_host_resolved = nullptr;
     }
 }
-std::string Connection::addrinfoToString(addrinfo *address)
+std::string Connection::addrinfoToString(const addrinfo *address) const
 {
     char host[NI_MAXHOST];
-    getnameinfo(address->ai_addr, address->ai_addrlen, host, NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
+    getnameinfo(address->ai_addr, address->ai_addrlen, host, NI_MAXHOST, nullptr, 0, NI_NUMERICHOST);
     return std::string(host);
 }
@@ -214,7 +214,7 @@ bool Connection::connectToAny()
         FD_SET(socket, &write_fds);
     }
-    int result = select(FD_SETSIZE, NULL, &write_fds, NULL, &timeout);
+    int result = select(FD_SETSIZE, nullptr, &write_fds, nullptr, &timeout);
     // Check if there was an error on select(). This is hard to recover from.
     if (result < 0)
@@ -24,15 +24,15 @@ class Connection
 {
 public:
     Connection(TrueMQTT::Client::LogLevel log_level,
-               const std::function<void(TrueMQTT::Client::LogLevel, std::string)> logger,
-               const std::function<void(TrueMQTT::Client::Error, std::string)> error_callback,
-               const std::function<void(std::string, std::string)> publish_callback,
-               const std::function<void(bool)> connection_change_callback,
+               const std::function<void(TrueMQTT::Client::LogLevel, std::string)> &logger,
+               const std::function<void(TrueMQTT::Client::Error, std::string)> &error_callback,
+               const std::function<void(std::string, std::string)> &publish_callback,
+               const std::function<void(bool)> &connection_change_callback,
                const std::string &host,
                int port);
     ~Connection();
-    void send(class Packet &packet);
+    void send(class Packet &packet) const;
 private:
     // Implemented in Connection.cpp
@@ -41,10 +41,10 @@ private:
     bool tryNextAddress();
     void connect(addrinfo *address);
     bool connectToAny();
-    std::string addrinfoToString(addrinfo *address);
+    std::string addrinfoToString(const addrinfo *address) const;
     // Implemented in Packet.cpp
-    ssize_t recv(char *buffer, size_t length);
+    ssize_t recv(char *buffer, size_t length) const;
     bool recvLoop();
     void sendConnect();
@@ -65,8 +65,8 @@ private:
     const std::function<void(std::string, std::string)> m_publish_callback;
     const std::function<void(bool)> m_connection_change_callback;
-    const std::string &m_host; ///< The hostname or IP address to connect to.
-    int m_port;                ///< The port to connect to.
+    const std::string m_host; ///< The hostname or IP address to connect to.
+    int m_port;               ///< The port to connect to.
     State m_state = State::RESOLVING;
     std::thread m_thread; ///< Current thread used to run this connection.
@@ -22,50 +22,60 @@
 #endif
 #if MIN_LOGGER_LEVEL >= LOGGER_LEVEL_ERROR
-#define LOG_ERROR(obj, x)                                    \
-    if (obj->log_level >= TrueMQTT::Client::LogLevel::ERROR) \
-    {                                                        \
-        obj->logger(TrueMQTT::Client::LogLevel::ERROR, x);   \
+#define LOG_ERROR(obj, x)                                        \
+    {                                                            \
+        if (obj->log_level >= TrueMQTT::Client::LogLevel::ERROR) \
+        {                                                        \
+            obj->logger(TrueMQTT::Client::LogLevel::ERROR, x);   \
+        }                                                        \
     }
 #else
 #define LOG_ERROR(obj, x)
 #endif
 #if MIN_LOGGER_LEVEL >= LOGGER_LEVEL_WARNING
-#define LOG_WARNING(obj, x)                                    \
-    if (obj->log_level >= TrueMQTT::Client::LogLevel::WARNING) \
-    {                                                          \
-        obj->logger(TrueMQTT::Client::LogLevel::WARNING, x);   \
+#define LOG_WARNING(obj, x)                                        \
+    {                                                              \
+        if (obj->log_level >= TrueMQTT::Client::LogLevel::WARNING) \
+        {                                                          \
+            obj->logger(TrueMQTT::Client::LogLevel::WARNING, x);   \
+        }                                                          \
     }
 #else
 #define LOG_WARNING(obj, x)
 #endif
 #if MIN_LOGGER_LEVEL >= LOGGER_LEVEL_INFO
-#define LOG_INFO(obj, x)                                    \
-    if (obj->log_level >= TrueMQTT::Client::LogLevel::INFO) \
-    {                                                       \
-        obj->logger(TrueMQTT::Client::LogLevel::INFO, x);   \
+#define LOG_INFO(obj, x)                                        \
+    {                                                           \
+        if (obj->log_level >= TrueMQTT::Client::LogLevel::INFO) \
+        {                                                       \
+            obj->logger(TrueMQTT::Client::LogLevel::INFO, x);   \
+        }                                                       \
     }
 #else
 #define LOG_INFO(obj, x)
 #endif
 #if MIN_LOGGER_LEVEL >= LOGGER_LEVEL_DEBUG
-#define LOG_DEBUG(obj, x)                                    \
-    if (obj->log_level >= TrueMQTT::Client::LogLevel::DEBUG) \
-    {                                                        \
-        obj->logger(TrueMQTT::Client::LogLevel::DEBUG, x);   \
+#define LOG_DEBUG(obj, x)                                        \
+    {                                                            \
+        if (obj->log_level >= TrueMQTT::Client::LogLevel::DEBUG) \
+        {                                                        \
+            obj->logger(TrueMQTT::Client::LogLevel::DEBUG, x);   \
+        }                                                        \
     }
 #else
 #define LOG_DEBUG(obj, x)
 #endif
 #if MIN_LOGGER_LEVEL >= LOGGER_LEVEL_TRACE
-#define LOG_TRACE(obj, x)                                    \
-    if (obj->log_level >= TrueMQTT::Client::LogLevel::TRACE) \
-    {                                                        \
-        obj->logger(TrueMQTT::Client::LogLevel::TRACE, x);   \
+#define LOG_TRACE(obj, x)                                        \
+    {                                                            \
+        if (obj->log_level >= TrueMQTT::Client::LogLevel::TRACE) \
+        {                                                        \
+            obj->logger(TrueMQTT::Client::LogLevel::TRACE, x);   \
+        }                                                        \
     }
 #else
 #define LOG_TRACE(obj, x)
@@ -65,7 +65,7 @@ public:
     void write_string(const std::string &str)
     {
-        write_uint16(str.size());
+        write_uint16(static_cast<uint16_t>(str.size()));
         write(str.c_str(), str.size());
     }
@@ -121,7 +121,7 @@ public:
     uint8_t m_flags;
 };
-ssize_t Connection::recv(char *buffer, size_t length)
+ssize_t Connection::recv(char *buffer, size_t length) const
 {
     // We idle-check every 100ms if we are requested to stop, as otherwise
     // this thread will block till the server disconnects us.
@@ -181,7 +181,7 @@ bool Connection::recvLoop()
         return false;
     }
-    Packet::PacketType packet_type = static_cast<Packet::PacketType>(packet_type_raw);
+    auto packet_type = static_cast<Packet::PacketType>(packet_type_raw);
     // Read the length of the packet. This is a bit slow to read, as only
     // after reading the byte we know if another byte follows.
@@ -322,7 +322,7 @@ bool Connection::recvLoop()
     return true;
 }
-void Connection::send(Packet &packet)
+void Connection::send(Packet &packet) const
 {
     LOG_TRACE(this, "Sending packet of type " + std::string(magic_enum::enum_name(packet.m_packet_type)) + " with flags " + std::to_string(packet.m_flags) + " and length " + std::to_string(packet.m_buffer.size()));