[GitHub] [nifi-minifi-cpp] hunyadi-dev edited a comment on pull request #866: MINIFICPP-1321 Support attribute strategies in MergeContent processor

[GitBox <gi...@apache.org>]

hunyadi-dev edited a comment on pull request #866:
URL: https://github.com/apache/nifi-minifi-cpp/pull/866#issuecomment-671932132


   Thanks for the modifications, I think [this one](https://github.com/apache/nifi-minifi-cpp/pull/866#discussion_r468394688) is still relevant: 
   
   Current:
   ```c++
   if (mergedAttributes.find(attr.first) != mergedAttributes.end()) {
     if (mergedAttributeIt->second != attr.second) {
       mergedAttributes.erase(attr.first);
       removed_attributes_.push_back(attr.first);
     }
   } else if (std::find(removed_attributes_.cbegin(), removed_attributes_.cend(), attr.first) == removed_attributes_.cend()) {
     mergedAttributes[attr.first] = attr.second;
   }
   ```
   
   Suggestion to avoid double map lookup:
   ```c++
   auto mergedAttributeFound = mergedAttributes.find(attr.first);
   if (mergedAttributeFound != mergedAttributes.end()) {
     if (mergedAttributeFound->second != attr.second) {
       mergedAttributes.erase(attr.first);
       removed_attributes_.push_back(attr.first);
     }
   } else if (std::find(removed_attributes_.cbegin(), removed_attributes_.cend(), attr.first) == removed_attributes_.cend()) {
     mergedAttributes.insert(attr);
   }
   ```
   
   You can further simplify this by checking the removed elements first:
   ```c++
   if(std::find(removed_attributes_.cbegin(), removed_attributes_.cend(), attr.first) != removed_attributes_.cend()) {
     continue;
   }
   std::map<std::string, std::string>::iterator insertion_res;
   bool insertion_happened;
   std::tie(insertion_res, insertion_happened) = mergedAttributes.insert(attr);
   if(!insertion_happened && insertion_res->second != attr.second) {
     mergedAttributes.erase(insertion_res);
     removed_attributes_.push_back(attr.first);
   }
   ```
   
   Another thing worth potentially improving is looping on `flowAttributes` by move, as we can consume it during the loop.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org