Modernize TriggerNames

FWCore/Common/interface/TriggerNames.h

@@ -45,7 +45,7 @@ for names changing more often than by run even in real data.
 #include "DataFormats/Provenance/interface/ParameterSetID.h"
 
 #include <string>
-#include <map>
+#include <utility>
 #include <vector>
 
 namespace edm {
@@ -54,14 +54,22 @@ namespace edm {
 
   class TriggerNames {
   public:
-    typedef std::vector<std::string> Strings;
-    typedef std::map<std::string, unsigned int> IndexMap;
+    using IndexMap = std::vector<std::pair<std::string_view, unsigned int>>;
+    using Strings = std::vector<std::string>;
 
     // Users should not construct these.  Instead they should
     // get a reference to the current one from the Event. See
     // comments above.
-    TriggerNames();
-    TriggerNames(edm::ParameterSet const& pset);
+    TriggerNames() = default;
+    explicit TriggerNames(edm::ParameterSet const& pset);
+
+    TriggerNames(TriggerNames const&);
+    TriggerNames(TriggerNames&&) = default;
+    TriggerNames& operator=(TriggerNames const&);
+    TriggerNames& operator=(TriggerNames&&) = default;
+
+    // called as part of reading back object from ROOT storage
+    void initializeTriggerIndex();
 
     Strings const& triggerNames() const;
 
@@ -70,10 +78,10 @@ namespace edm {
 
     // If the input name is not known, this returns a value
     // equal to the size.
-    unsigned int triggerIndex(std::string const& name) const;
+    unsigned int triggerIndex(std::string_view name) const;
 
     // The number of trigger names.
-    Strings::size_type size() const;
+    std::size_t size() const;
 
     // Can be used to quickly compare two TriggerNames objects
     // to see whether or not they contain the same names.

FWCore/Common/src/TriggerNames.cc

@@ -1,34 +1,62 @@
 
 #include "FWCore/Common/interface/TriggerNames.h"
 #include "FWCore/ParameterSet/interface/ParameterSet.h"
+#include <algorithm>
 
+namespace {
+  struct PairSort {
+    bool operator()(std::pair<std::string_view, unsigned int> const& iLHS,
+                    std::pair<std::string_view, unsigned int> const& iRHS) const {
+      return iLHS.first < iRHS.first;
+    }
+    bool operator()(std::string_view iLHS, std::pair<std::string_view, unsigned int> const& iRHS) const {
+      return iLHS < iRHS.first;
+    }
+    bool operator()(std::pair<std::string_view, unsigned int> const& iLHS, std::string_view iRHS) const {
+      return iLHS.first < iRHS;
+    }
+  };
+}  // namespace
 namespace edm {
 
-  TriggerNames::TriggerNames() {}
+  TriggerNames::TriggerNames(edm::ParameterSet const& pset)
+      : psetID_{pset.id()}, triggerNames_{pset.getParameter<Strings>("@trigger_paths")} {
+    initializeTriggerIndex();
+  }
 
-  TriggerNames::TriggerNames(edm::ParameterSet const& pset) {
-    triggerNames_ = pset.getParameter<Strings>("@trigger_paths");
+  TriggerNames::TriggerNames(TriggerNames const& iOther)
+      : psetID_{iOther.psetID_}, triggerNames_{iOther.triggerNames_} {
+    initializeTriggerIndex();
+  }
+
+  TriggerNames& TriggerNames::operator=(TriggerNames const& iOther) {
+    TriggerNames temp(iOther);
+    *this = std::move(temp);
+    return *this;
+  }
 
+  void TriggerNames::initializeTriggerIndex() {
     unsigned int index = 0;
-    for (Strings::const_iterator iName = triggerNames_.begin(), iEnd = triggerNames_.end(); iName != iEnd;
-         ++iName, ++index) {
-      indexMap_[*iName] = index;
+    indexMap_.reserve(triggerNames_.size());
+    for (auto const& name : triggerNames_) {
+      indexMap_.emplace_back(name, index);
+      ++index;
     }
-    psetID_ = pset.id();
+    std::sort(indexMap_.begin(), indexMap_.end(), PairSort());
   }
 
   TriggerNames::Strings const& TriggerNames::triggerNames() const { return triggerNames_; }
 
   std::string const& TriggerNames::triggerName(unsigned int index) const { return triggerNames_.at(index); }
 
-  unsigned int TriggerNames::triggerIndex(const std::string& name) const {
-    IndexMap::const_iterator const pos = indexMap_.find(name);
-    if (pos == indexMap_.end())
+  unsigned int TriggerNames::triggerIndex(std::string_view name) const {
+    auto found = std::equal_range(indexMap_.begin(), indexMap_.end(), name, PairSort());
+    if (found.first == found.second)
       return indexMap_.size();
-    return pos->second;
+    return found.first->second;
   }
 
-  TriggerNames::Strings::size_type TriggerNames::size() const { return triggerNames_.size(); }
+  std::size_t TriggerNames::size() const { return triggerNames_.size(); }
 
   ParameterSetID const& TriggerNames::parameterSetID() const { return psetID_; }
 }  // namespace edm

FWCore/Common/src/classes_def.xml

@@ -8,9 +8,16 @@
   <class name="edm::RunBase" ClassVersion="10">
    <version ClassVersion="10" checksum="26549798"/>
   </class>
-  <class name="edm::TriggerNames" ClassVersion="10">
+  <class name="edm::TriggerNames" ClassVersion="11">
+   <field name="indexMap_" transient="true"/>
    <version ClassVersion="10" checksum="2872088986"/>
+   <version ClassVersion="11" checksum="1892641650"/>
   </class>
+  <ioread sourceClass="edm::TriggerNames" targetClass="edm::TriggerNames" version="[1-]" source="" target="indexMap_">
+  <![CDATA[
+   newObj->initializeTriggerIndex();
+  ]]>
+  </ioread>
   <class name="edm::TriggerResultsByName" ClassVersion="11">
    <version ClassVersion="11" checksum="3334716878"/>
    <version ClassVersion="10" checksum="1282053882"/>

FWCore/Common/test/BuildFile.xml

@@ -0,0 +1,5 @@
+
+<bin file="test_catch2_*.cc" name="testFWCoreCommonCatch2">
+  <use name="FWCore/Common"/>
+  <use name="catch2"/>
+</bin>

FWCore/Common/test/test_catch2_TriggerNames.cc

@@ -0,0 +1,140 @@
+#define CATCH_CONFIG_MAIN
+#include "catch.hpp"
+#include "FWCore/Common/interface/TriggerNames.h"
+#include "FWCore/ParameterSet/interface/ParameterSet.h"
+#include <memory>
+
+TEST_CASE("Test TriggerNames", "[TriggerNames]") {
+  edm::ParameterSet pset;
+  const std::vector<std::string> names = {{"b1"}, {"b2"}, {"a1"}, {"z5"}};
+  pset.addParameter<std::vector<std::string>>("@trigger_paths", names);
+  pset.registerIt();
+
+  SECTION("Default constructed") {
+    edm::TriggerNames defaultConst;
+
+    REQUIRE(defaultConst.size() == 0);
+    REQUIRE(defaultConst.triggerNames().empty());
+    REQUIRE(defaultConst.triggerIndex("not here") == defaultConst.size());
+    REQUIRE_THROWS_AS(defaultConst.triggerName(0), std::exception);
+  }
+
+  SECTION("No names") {
+    edm::ParameterSet pset;
+    const std::vector<std::string> names;
+    pset.addParameter<std::vector<std::string>>("@trigger_paths", names);
+    pset.registerIt();
+
+    edm::TriggerNames noNames(pset);
+
+    REQUIRE(noNames.size() == 0);
+    REQUIRE(noNames.triggerNames().empty());
+    REQUIRE(noNames.parameterSetID() == pset.id());
+    REQUIRE(noNames.triggerIndex("not here") == noNames.size());
+    REQUIRE_THROWS_AS(noNames.triggerName(0), std::exception);
+  }
+
+  SECTION("multiple names") {
+    edm::TriggerNames tNames(pset);
+
+    REQUIRE(tNames.size() == names.size());
+    REQUIRE(tNames.triggerNames() == names);
+    REQUIRE(tNames.parameterSetID() == pset.id());
+    REQUIRE(tNames.triggerIndex("not here") == tNames.size());
+    REQUIRE(tNames.triggerIndex("b1") == 0);
+    REQUIRE(tNames.triggerIndex("b2") == 1);
+    REQUIRE(tNames.triggerIndex("a1") == 2);
+    REQUIRE(tNames.triggerIndex("z5") == 3);
+    REQUIRE(tNames.triggerName(0) == "b1");
+    REQUIRE(tNames.triggerName(1) == "b2");
+    REQUIRE(tNames.triggerName(2) == "a1");
+    REQUIRE(tNames.triggerName(3) == "z5");
+    REQUIRE_THROWS_AS(tNames.triggerName(names.size()), std::exception);
+  }
+
+  SECTION("copy constructor") {
+    auto temp = std::make_unique<edm::TriggerNames>(pset);
+
+    edm::TriggerNames tNames(*temp);
+    temp.release();
+
+    REQUIRE(tNames.size() == names.size());
+    REQUIRE(tNames.triggerNames() == names);
+    REQUIRE(tNames.parameterSetID() == pset.id());
+    REQUIRE(tNames.triggerIndex("not here") == tNames.size());
+    REQUIRE(tNames.triggerIndex("b1") == 0);
+    REQUIRE(tNames.triggerIndex("b2") == 1);
+    REQUIRE(tNames.triggerIndex("a1") == 2);
+    REQUIRE(tNames.triggerIndex("z5") == 3);
+    REQUIRE(tNames.triggerName(0) == "b1");
+    REQUIRE(tNames.triggerName(1) == "b2");
+    REQUIRE(tNames.triggerName(2) == "a1");
+    REQUIRE(tNames.triggerName(3) == "z5");
+    REQUIRE_THROWS_AS(tNames.triggerName(names.size()), std::exception);
+  }
+
+  SECTION("move constructor") {
+    auto temp = std::make_unique<edm::TriggerNames>(pset);
+
+    edm::TriggerNames tNames(std::move(*temp));
+    temp.release();
+
+    REQUIRE(tNames.size() == names.size());
+    REQUIRE(tNames.triggerNames() == names);
+    REQUIRE(tNames.parameterSetID() == pset.id());
+    REQUIRE(tNames.triggerIndex("not here") == tNames.size());
+    REQUIRE(tNames.triggerIndex("b1") == 0);
+    REQUIRE(tNames.triggerIndex("b2") == 1);
+    REQUIRE(tNames.triggerIndex("a1") == 2);
+    REQUIRE(tNames.triggerIndex("z5") == 3);
+    REQUIRE(tNames.triggerName(0) == "b1");
+    REQUIRE(tNames.triggerName(1) == "b2");
+    REQUIRE(tNames.triggerName(2) == "a1");
+    REQUIRE(tNames.triggerName(3) == "z5");
+    REQUIRE_THROWS_AS(tNames.triggerName(names.size()), std::exception);
+  }
+
+  SECTION("operator=") {
+    auto temp = std::make_unique<edm::TriggerNames>(pset);
+
+    edm::TriggerNames tNames;
+    tNames = (*temp);
+    temp.release();
+
+    REQUIRE(tNames.size() == names.size());
+    REQUIRE(tNames.triggerNames() == names);
+    REQUIRE(tNames.parameterSetID() == pset.id());
+    REQUIRE(tNames.triggerIndex("not here") == tNames.size());
+    REQUIRE(tNames.triggerIndex("b1") == 0);
+    REQUIRE(tNames.triggerIndex("b2") == 1);
+    REQUIRE(tNames.triggerIndex("a1") == 2);
+    REQUIRE(tNames.triggerIndex("z5") == 3);
+    REQUIRE(tNames.triggerName(0) == "b1");
+    REQUIRE(tNames.triggerName(1) == "b2");
+    REQUIRE(tNames.triggerName(2) == "a1");
+    REQUIRE(tNames.triggerName(3) == "z5");
+    REQUIRE_THROWS_AS(tNames.triggerName(names.size()), std::exception);
+  }
+
+  SECTION("operator= with move") {
+    auto temp = std::make_unique<edm::TriggerNames>(pset);
+
+    edm::TriggerNames tNames;
+    tNames = std::move(*temp);
+    temp.release();
+
+    REQUIRE(tNames.size() == names.size());
+    REQUIRE(tNames.triggerNames() == names);
+    REQUIRE(tNames.parameterSetID() == pset.id());
+    REQUIRE(tNames.triggerIndex("not here") == tNames.size());
+    REQUIRE(tNames.triggerIndex("b1") == 0);
+    REQUIRE(tNames.triggerIndex("b2") == 1);
+    REQUIRE(tNames.triggerIndex("a1") == 2);
+    REQUIRE(tNames.triggerIndex("z5") == 3);
+    REQUIRE(tNames.triggerName(0) == "b1");
+    REQUIRE(tNames.triggerName(1) == "b2");
+    REQUIRE(tNames.triggerName(2) == "a1");
+    REQUIRE(tNames.triggerName(3) == "z5");
+    REQUIRE_THROWS_AS(tNames.triggerName(names.size()), std::exception);
+  }
+}