[sonar] follow recommandations to improve code maintainability

Author: frs69wq

include/dtlmod/Metadata.hpp

@@ -19,18 +19,24 @@ class Variable;
 
 /// \cond EXCLUDE_FROM_DOCUMENTATION
 class Metadata {
+  friend Variable;
   Variable* variable_;
 
-public:
-  explicit Metadata(Variable* variable) : variable_(variable) {}
+  std::map<unsigned int,                                          // Transaction id
+    std::map<std::pair<std::vector<size_t>, std::vector<size_t>>, // starts and counts
+             std::pair<std::string, sg4::ActorPtr>,               // filename and publisher
+             std::less<>>,
+    std::less<>> transaction_infos_;
 
-  std::map<unsigned int,                                                 // Transaction id
-           std::map<std::pair<std::vector<size_t>, std::vector<size_t>>, // starts and counts
-                    std::pair<std::string, sg4::ActorPtr>,               // filename and publisher
-                    std::less<>>,
-           std::less<>>
-      transaction_infos_;
+protected:
+  const std::map<std::pair<std::vector<size_t>, std::vector<size_t>>, 
+             std::pair<std::string, sg4::ActorPtr>, std::less<>>& get_blocks_for_transaction(int id) 
+    { return transaction_infos_[id]; }
+  void add_transaction(int id, const std::vector<size_t>& start, const std::vector<size_t>& count,
+                       const std::string& filename, sg4::ActorPtr publisher); 
 
+public:
+  explicit Metadata(Variable* variable) : variable_(variable) {}
   int get_current_transaction() const { return transaction_infos_.empty() ? -1 : (transaction_infos_.rbegin())->first; }
   void export_to_file(std::ofstream& ostream) const;
 };

include/dtlmod/StagingTransport.hpp

@@ -16,6 +16,7 @@ class StagingTransport : public Transport {
   using Transport::Transport;
   friend StagingEngine;
   std::unordered_map<std::string, sg4::MessageQueue*> publisher_put_requests_mq_;
+  std::unordered_map<std::string, sg4::ActivitySet> pending_put_requests_;
 
 protected:
   void add_publisher(unsigned long publisher_id) override;
@@ -26,9 +27,12 @@ class StagingTransport : public Transport {
   // Create a message queue to receive request for variable pieces from subscribers
   void set_publisher_put_requests_mq(const std::string& publisher_name);
   [[nodiscard]] sg4::MessageQueue* get_publisher_put_requests_mq(const std::string& publisher_name) const;
-
-public:
-  std::unordered_map<std::string, sg4::ActivitySet> pending_put_requests;
+  [[nodiscard]] bool pending_put_requests_exist_for(const std::string& pub_name)
+  { return not pending_put_requests_[pub_name].empty(); }
+  [[nodiscard]] sg4::ActivityPtr wait_any_pending_put_request_for(const std::string& pub_name)
+  { return pending_put_requests_[pub_name].wait_any(); }
+  
+  public:
   ~StagingTransport() override = default;
   void put(std::shared_ptr<Variable> var, size_t /*simulated_size_in_bytes*/) override;
   void get(std::shared_ptr<Variable> var) override;

src/DTL.cpp

@@ -8,7 +8,6 @@
 
 #include "dtlmod/DTL.hpp"
 #include "dtlmod/DTLException.hpp"
-#define JSON_USE_IMPLICIT_CONVERSIONS 0
 #include <fstream>
 #include <nlohmann/json.hpp>
 

src/FileTransport.cpp

@@ -41,7 +41,7 @@ void FileTransport::put(std::shared_ptr<Variable> var, size_t size)
   var->add_transaction_metadata(tid, self, file->get_path());
 
   XBT_DEBUG("Actor '%s' is writing %lu bytes into file '%s'", self->get_cname(), size, file->get_path().c_str());
-  to_write_in_transaction_[self].emplace_back(std::make_pair(file, size));
+  to_write_in_transaction_[self].emplace_back(file, size);
 }
 
 void FileTransport::close_pub_files() const
@@ -71,7 +71,7 @@ void FileTransport::get(std::shared_ptr<Variable> var)
       XBT_DEBUG("Actor '%s' is opening file '%s'", self->get_cname(), filename.c_str());
       auto file = fs->open(filename, "r");
       // Keep track of what to read from this file for this get
-      to_read_in_transaction_[self].emplace_back(std::make_pair(file, size));
+      to_read_in_transaction_[self].emplace_back(file, size);
     }
   }
 }

src/Metadata.cpp

@@ -11,6 +11,13 @@ XBT_LOG_EXTERNAL_DEFAULT_CATEGORY(dtlmod);
 
 namespace dtlmod {
 /// \cond EXCLUDE_FROM_DOCUMENTATION
+void Metadata::add_transaction(int id, const std::vector<size_t>& start, const std::vector<size_t>& count,
+                       const std::string& location, sg4::ActorPtr publisher)
+{
+  auto start_and_count = std::make_pair(start, count);
+  transaction_infos_[id][start_and_count] = std::make_pair(location, publisher);
+}
+
 void Metadata::export_to_file(std::ofstream& ostream) const
 {
   XBT_DEBUG("Variable %s:", variable_->get_cname());

src/StagingMboxTransport.cpp

@@ -31,8 +31,8 @@ void StagingMboxTransport::get_requests_and_do_put(sg4::ActorPtr publisher)
   auto pub_name = publisher->get_name();
   // Wait for the reception of the messages. If something is requested, post a put in the mailbox for the
   // corresponding publisher-subscriber couple
-  while (not pending_put_requests[pub_name].empty()) {
-    auto request     = boost::static_pointer_cast<sg4::Mess>(pending_put_requests[pub_name].wait_any());
+  while (pending_put_requests_exist_for(pub_name)) {
+    auto request     = boost::static_pointer_cast<sg4::Mess>(wait_any_pending_put_request_for(pub_name));
     const auto* subscriber = request->get_sender();
     auto* req_size   = static_cast<size_t*>(request->get_payload());
     if (*req_size > 0) {

src/StagingMqTransport.cpp

@@ -30,8 +30,8 @@ void StagingMqTransport::get_requests_and_do_put(sg4::ActorPtr publisher)
   auto pub_name = publisher->get_name();
   // Wait for the reception of the messages. If something is requested, post a put in the message queue for the
   // corresponding publisher-subscriber couple
-  while (not pending_put_requests[pub_name].empty()) {
-    auto request     = boost::static_pointer_cast<sg4::Mess>(pending_put_requests[pub_name].wait_any());
+  while (pending_put_requests_exist_for(pub_name)) {
+    auto request     = boost::static_pointer_cast<sg4::Mess>(wait_any_pending_put_request_for(pub_name));
     const auto* subscriber = request->get_sender();
     auto* req_size   = static_cast<size_t*>(request->get_payload());
     if (*req_size > 0) {

src/StagingTransport.cpp

@@ -44,8 +44,8 @@ void StagingTransport::put(std::shared_ptr<Variable> var, size_t simulated_size_
   // Each Subscriber will send a put request to each publisher in the Stream. They can request for a certain size if
   // they need something from this publisher or 0 otherwise.
   // Start with posting all asynchronous gets and creating an ActivitySet.
-  for (unsigned int i = 0; i < e->get_num_subscribers(); i++)
-    pending_put_requests[pub_name].push(get_publisher_put_requests_mq(pub_name)->get_async());
+  for (size_t i = 0; i < e->get_num_subscribers(); i++)
+    pending_put_requests_[pub_name].push(get_publisher_put_requests_mq(pub_name)->get_async());
 }
 
 void StagingTransport::get(std::shared_ptr<Variable> var)

src/Variable.cpp

@@ -74,8 +74,7 @@ const std::pair<unsigned int, unsigned int>& Variable::get_subscriber_transactio
 void Variable::add_transaction_metadata(unsigned int transaction_id, sg4::ActorPtr publisher,
                                         const std::string& location)
 {
-  auto start_and_count = std::make_pair(local_start_[publisher], local_count_[publisher]);
-  metadata_->transaction_infos_[transaction_id][start_and_count] = std::make_pair(location, publisher);
+  metadata_->add_transaction(transaction_id, local_start_[publisher], local_count_[publisher], location, publisher);
 }
 
 std::vector<std::pair<std::string, sg_size_t>> Variable::get_sizes_to_get_per_block(unsigned int transaction_id,
@@ -84,7 +83,7 @@ std::vector<std::pair<std::string, sg_size_t>> Variable::get_sizes_to_get_per_bl
 {
   std::vector<std::pair<std::string, sg_size_t>> get_sizes_per_block;
   // TODO add sanity checks
-  auto blocks = metadata_->transaction_infos_[transaction_id];
+  auto blocks = metadata_->get_blocks_for_transaction(transaction_id);
   XBT_DEBUG("%zu block(s) to check for transaction %u", blocks.size(), transaction_id);
   for (const auto& [block_info, location] : blocks) {
     size_t size_to_get              = element_size_;
@@ -98,18 +97,15 @@ std::vector<std::pair<std::string, sg_size_t>> Variable::get_sizes_to_get_per_bl
                 block_count[i]);
       size_t size_in_dim = 0;
       bool element_found = false;
-      if (start[i] <= block_start[i]) {
-        if ((block_start[i] - start[i]) <= count[i]) {
+      if (start[i] <= block_start[i] && (block_start[i] - start[i]) <= count[i]) {
           size_in_dim   = std::min(block_count[i], count[i] - (block_start[i] - start[i]));
           element_found = true;
-        }
-      } else {
-        if ((start[i] - block_start[i]) <= block_count[i]) {
+      }
+      if (start[i] > block_start[i] && (start[i] - block_start[i]) <= block_count[i]) {
           size_in_dim   = std::min(count[i], block_count[i] - (start[i] - block_start[i]));
           element_found = true;
-        }
       }
-
+      
       if (element_found && size_in_dim > 0) {
         something_to_get[i] = true;
         XBT_DEBUG("Mutiply size to read by %zu elements", size_in_dim);
@@ -119,7 +115,7 @@ std::vector<std::pair<std::string, sg_size_t>> Variable::get_sizes_to_get_per_bl
 
     if (std::all_of(something_to_get.begin(), something_to_get.end(), [](bool v) { return v; })) {
       XBT_DEBUG("Total size to read from %s: %zu)", where.c_str(), size_to_get);
-      get_sizes_per_block.emplace_back(std::make_pair(where, size_to_get));
+      get_sizes_per_block.emplace_back(where, size_to_get);
     }
   }
   return get_sizes_per_block;