Moving to std::copy, addressing comments, using catch2 for tests.

RecoTracker/TkSeedingLayers/interface/SeedingHitSet.h

@@ -26,29 +26,36 @@ class SeedingHitSet {
       : SeedingHitSet({one, two, three, four}) {}
 
   SeedingHitSet(const std::vector<ConstRecHitPointer> &hits) {
-    theRecHits.reserve(hits.size());
-    std::copy_n(
-        hits.begin(), std::find(hits.begin(), hits.end(), nullPtr()) - hits.begin(), std::back_inserter(theRecHits));
-    theSize = theRecHits.size() > 1 ? theRecHits.size() : 0;
+    if (hits.size() >= 2) {
+      auto end = std::find(hits.begin(), hits.end(), nullPtr());
+      auto size = std::distance(hits.begin(), end);
+      if (size >= 2) {
+        theRecHits.reserve(size);
+        std::copy(hits.begin(), end, std::back_inserter(theRecHits));
+      }
+    }
   }
 
-  SeedingHitSet(const std::initializer_list<ConstRecHitPointer> &hits) : SeedingHitSet() {
-    theRecHits.reserve(hits.size());
-    std::copy_n(
-        hits.begin(), std::find(hits.begin(), hits.end(), nullPtr()) - hits.begin(), std::back_inserter(theRecHits));
-    theSize = theRecHits.size() > 1 ? theRecHits.size() : 0;
+  SeedingHitSet(const std::initializer_list<ConstRecHitPointer> &hits) {
+    if (hits.size() >= 2) {
+      auto end = std::find(hits.begin(), hits.end(), nullPtr());
+      auto size = std::distance(hits.begin(), end);
+      if (size >= 2) {
+        theRecHits.reserve(size);
+        std::copy(hits.begin(), end, std::back_inserter(theRecHits));
+      }
+    }
   }
 
   ConstRecHitPointer const *data() const { return theRecHits.data(); }
 
-  unsigned int size() const { return theSize; }
+  unsigned int size() const { return theRecHits.size(); }
 
   ConstRecHitPointer get(unsigned int i) const { return theRecHits[i]; }
   ConstRecHitPointer operator[](unsigned int i) const { return theRecHits[i]; }
 
 protected:
   std::vector<ConstRecHitPointer> theRecHits;
-  unsigned int theSize = 0;
 };
 
 #endif

RecoTracker/TkSeedingLayers/test/BuildFile.xml

@@ -2,7 +2,11 @@
 <use name="FWCore/Framework"/>
 <use name="FWCore/ParameterSet"/>
 <use name="TrackingTools/TransientTrackingRecHit"/>
-<bin file="SeedingHitSet_t.cpp"/>
 <library file="*.cc" name="RecoTrackerTkSeedingLayersTests">
   <flags EDM_PLUGIN="1"/>
 </library>
+
+<bin file="test_catch2_*.cpp" name="test_catch2_RECOTRACKER_TKSEEDINGLAYERS">
+  <use name="DataFormats/TrackerRecHit2D"/>
+  <use name="catch2"/>
+</bin>

RecoTracker/TkSeedingLayers/test/test_catch2_SeedingHitSet.cpp

@@ -0,0 +1,74 @@
+#include "RecoTracker/TkSeedingLayers/interface/SeedingHitSet.h"
+#include "DataFormats/TrackerRecHit2D/interface/TrackerSingleRecHit.h"
+#include "DataFormats/TrackerRecHit2D/interface/SiPixelRecHit.h"
+#include "DataFormats/TrackerRecHit2D/interface/SiStripRecHit2D.h"
+#include "DataFormats/TrackerRecHit2D/interface/SiStripRecHit1D.h"
+#include "DataFormats/TrackerRecHit2D/interface/MTDTrackingRecHit.h"
+#include "DataFormats/TrackerRecHit2D/interface/Phase2TrackerRecHit1D.h"
+
+#include "catch.hpp"
+
+#include <iostream>
+
+TEST_CASE("SeedingHitSet testing constructors", "[SeedingHitSet]") {
+  using Hit = SeedingHitSet::RecHit;
+  using HitPointer = SeedingHitSet::ConstRecHitPointer;
+
+  HitPointer hitOne = new SiPixelRecHit();
+  HitPointer hitTwo = new MTDTrackingRecHit();
+  HitPointer hitThree = new SiStripRecHit1D();
+  HitPointer hitFour = new SiStripRecHit2D();
+  HitPointer hitFive = new Phase2TrackerRecHit1D();
+
+  HitPointer HIT_NULL = nullptr;
+
+  SECTION("Check vector constructor") {
+    std::vector<HitPointer> containerOne = {hitOne, hitTwo, hitThree, hitFour, hitFive};
+    std::vector<HitPointer> containerTwo = {hitOne, hitTwo, hitThree, HIT_NULL, hitFive};
+    std::vector<HitPointer> containerThree = {hitOne, HIT_NULL, hitThree, hitFour, hitFive};
+
+    SeedingHitSet vecOne(containerOne);
+    REQUIRE(vecOne.size() == containerOne.size());
+    SeedingHitSet vecTwo(containerTwo);
+    REQUIRE(vecTwo.size() == 3);
+    SeedingHitSet vecThree(containerThree);
+    REQUIRE(vecThree.size() == 0);
+  }
+
+  SECTION("Check braced constructor") {
+    SeedingHitSet listOne({hitThree, hitFive, hitFour});
+    REQUIRE(listOne.size() == 3);
+    SeedingHitSet listTwo({HIT_NULL, hitTwo, hitFour});
+    REQUIRE(listTwo.size() == 0);
+    SeedingHitSet listThree({hitTwo, hitFour, hitOne, hitFive, HIT_NULL});
+    REQUIRE(listThree.size() == 4);
+  }
+
+  SECTION("Check two hits constructor") {
+    SeedingHitSet twoHitsOne(hitOne, hitTwo);
+    REQUIRE(twoHitsOne.size() == 2);
+    SeedingHitSet twoHitsTwo(HIT_NULL, hitTwo);
+    REQUIRE(twoHitsTwo.size() == 0);
+    SeedingHitSet twoHitsThree(hitOne, HIT_NULL);
+    REQUIRE(twoHitsThree.size() == 0);
+  }
+
+  SECTION("Check three hits constructor") {
+    SeedingHitSet threeHitsOne(hitOne, hitTwo, hitThree);
+    REQUIRE(threeHitsOne.size() == 3);
+    SeedingHitSet threeHitsTwo(hitOne, hitTwo, HIT_NULL);
+    REQUIRE(threeHitsTwo.size() == 2);
+    SeedingHitSet threeHitsThree(hitOne, HIT_NULL, hitThree);
+    REQUIRE(threeHitsThree.size() == 0);
+  }
+  SECTION("Check four hits constructor") {
+    SeedingHitSet fourHitsOne(hitOne, hitTwo, hitThree, hitFour);
+    REQUIRE(fourHitsOne.size() == 4);
+    SeedingHitSet fourHitsTwo(hitOne, hitTwo, HIT_NULL, hitFour);
+    REQUIRE(fourHitsTwo.size() == 2);
+    SeedingHitSet fourHitsThree(hitOne, hitTwo, hitThree, HIT_NULL);
+    REQUIRE(fourHitsThree.size() == 3);
+    SeedingHitSet fourHitsFour(hitOne, HIT_NULL, hitThree, hitFour);
+    REQUIRE(fourHitsFour.size() == 0);
+  }
+}

RecoTracker/TkSeedingLayers/test/test_catch2_main.cpp

@@ -0,0 +1,2 @@
+#define CATCH_CONFIG_MAIN  // This tells Catch to provide a main() - only do this in one cpp file
+#include "catch.hpp"