DataSet queries for cellset and coordinate index won't trow

Queries for cellset and coordinates by name will not throw exceptions
when looking just for the index value. Instead they will return -1

docs/changelog/dataset-no-throw-on-index-queries.md

@@ -0,0 +1,11 @@
+# DataSet queries for CellSet and Coordinate System Indices don't throw
+
+Asking for the index of a `vtkm::cont::CellSet` or `vtkm::cont::CoordinateSystem` by
+name now returns a `-1` when no matching item has been found instead of throwing
+an exception.
+
+This was done to make the interface of `vtkm::cont::DataSet` to follow the guideline
+"Only unrepresentable things should raise exceptions". The index of a non-existent item
+is representable by `-1` and therefore we shouldn't throw, like wise the methods that return
+references can still throw exceptions as you can't have a reference to an non-existent item.
+

vtkm/cont/DataSet.cxx

@@ -55,30 +55,67 @@ const vtkm::cont::CoordinateSystem& DataSet::GetCoordinateSystem(vtkm::Id index)
 
 vtkm::Id DataSet::GetCoordinateSystemIndex(const std::string& name) const
 {
-  bool found;
+  vtkm::Id index = -1;
-  vtkm::Id index = this->FindCoordinateSystemIndex(name, found);
+  for (auto i = this->CoordSystems.begin(); i != this->CoordSystems.end(); ++i)
-  if (found)
   {
-    return index;
+    if (i->GetName() == name)
+    {
+      index = static_cast<vtkm::Id>(std::distance(this->CoordSystems.begin(), i));
+      break;
+    }
   }
-  else
+  return index;
+}
+
+const vtkm::cont::CoordinateSystem& DataSet::GetCoordinateSystem(const std::string& name) const
+{
+  vtkm::Id index = this->GetCoordinateSystemIndex(name);
+  if (index < 0)
   {
-    throw vtkm::cont::ErrorBadValue("No coordinate system with requested name");
+    std::string error_message("No coordinate system with the name " + name +
+                              " valid names are: \n");
+    for (const auto& cs : this->CoordSystems)
+    {
+      error_message += cs.GetName() + "\n";
+    }
+    throw vtkm::cont::ErrorBadValue(error_message);
   }
+  return this->GetCoordinateSystem(index);
+}
+
+const vtkm::cont::DynamicCellSet& DataSet::GetCellSet(vtkm::Id index) const
+{
+  VTKM_ASSERT((index >= 0) && (index < this->GetNumberOfCellSets()));
+  return this->CellSets[static_cast<std::size_t>(index)];
 }
 
 vtkm::Id DataSet::GetCellSetIndex(const std::string& name) const
 {
-  bool found;
+  vtkm::Id index = -1;
-  vtkm::Id index = this->FindCellSetIndex(name, found);
+  for (auto i = this->CellSets.begin(); i != this->CellSets.end(); ++i)
-  if (found)
   {
-    return index;
+    if (i->GetName() == name)
+    {
+      index = static_cast<vtkm::Id>(std::distance(this->CellSets.begin(), i));
+      break;
+    }
   }
-  else
+  return index;
+}
+
+const vtkm::cont::DynamicCellSet& DataSet::GetCellSet(const std::string& name) const
+{
+  vtkm::Id index = this->GetCellSetIndex(name);
+  if (index < 0)
   {
-    throw vtkm::cont::ErrorBadValue("No cell set with requested name");
+    std::string error_message("No cell set with the name " + name + " valid names are: \n");
+    for (const auto& cs : this->CellSets)
+    {
+      error_message += cs.GetName() + "\n";
+    }
+    throw vtkm::cont::ErrorBadValue(error_message);
   }
+  return this->GetCellSet(index);
 }
 
 void DataSet::PrintSummary(std::ostream& out) const
@@ -121,34 +158,5 @@ vtkm::Id DataSet::FindFieldIndex(const std::string& name,
   return -1;
 }
 
-
-vtkm::Id DataSet::FindCoordinateSystemIndex(const std::string& name, bool& found) const
-{
-  for (std::size_t index = 0; index < this->CoordSystems.size(); ++index)
-  {
-    if (this->CoordSystems[index].GetName() == name)
-    {
-      found = true;
-      return static_cast<vtkm::Id>(index);
-    }
-  }
-  found = false;
-  return -1;
-}
-
-vtkm::Id DataSet::FindCellSetIndex(const std::string& name, bool& found) const
-{
-  for (std::size_t index = 0; index < static_cast<size_t>(this->GetNumberOfCellSets()); ++index)
-  {
-    if (this->CellSets[index].GetName() == name)
-    {
-      found = true;
-      return static_cast<vtkm::Id>(index);
-    }
-  }
-  found = false;
-  return -1;
-}
-
 } // namespace cont
 } // namespace vtkm

vtkm/cont/DataSet.h

@@ -60,11 +60,16 @@ public:
     return found;
   }
 
+
+  /// Returns the first field that matches the provided name and association
+  /// Will return -1 if no match is found
   VTKM_CONT
   vtkm::Id GetFieldIndex(
     const std::string& name,
     vtkm::cont::Field::Association assoc = vtkm::cont::Field::Association::ANY) const;
 
+  /// Returns the first field that matches the provided name and association
+  /// Will throw an exception if no match is found
   VTKM_CONT
   const vtkm::cont::Field& GetField(
     const std::string& name,
@@ -73,12 +78,16 @@ public:
     return this->GetField(this->GetFieldIndex(name, assoc));
   }
 
+  /// Returns the first cell field that matches the provided name.
+  /// Will throw an exception if no match is found
   VTKM_CONT
   const vtkm::cont::Field& GetCellField(const std::string& name) const
   {
     return this->GetField(name, vtkm::cont::Field::Association::CELL_SET);
   }
 
+  /// Returns the first point field that matches the provided name.
+  /// Will throw an exception if no match is found
   VTKM_CONT
   const vtkm::cont::Field& GetPointField(const std::string& name) const
   {
@@ -91,25 +100,25 @@ public:
     this->CoordSystems.push_back(cs);
   }
 
-  VTKM_CONT
-  const vtkm::cont::CoordinateSystem& GetCoordinateSystem(vtkm::Id index = 0) const;
-
   VTKM_CONT
   bool HasCoordinateSystem(const std::string& name) const
   {
-    bool found;
+    return this->GetCoordinateSystemIndex(name) >= 0;
-    this->FindCoordinateSystemIndex(name, found);
-    return found;
   }
 
+  VTKM_CONT
+  const vtkm::cont::CoordinateSystem& GetCoordinateSystem(vtkm::Id index = 0) const;
+
+  /// Returns the index for the first CoordinateSystem whose
+  /// name matches the provided string.
+  /// Will return -1 if no match is found
   VTKM_CONT
   vtkm::Id GetCoordinateSystemIndex(const std::string& name) const;
 
+  /// Returns the first CoordinateSystem that matches the provided name.
+  /// Will throw an exception if no match is found
   VTKM_CONT
-  const vtkm::cont::CoordinateSystem& GetCoordinateSystem(const std::string& name) const
+  const vtkm::cont::CoordinateSystem& GetCoordinateSystem(const std::string& name) const;
-  {
-    return this->GetCoordinateSystem(this->GetCoordinateSystemIndex(name));
-  }
 
   VTKM_CONT
   void AddCellSet(const vtkm::cont::DynamicCellSet& cellSet) { this->CellSets.push_back(cellSet); }
@@ -122,28 +131,24 @@ public:
   }
 
   VTKM_CONT
-  vtkm::cont::DynamicCellSet GetCellSet(vtkm::Id index = 0) const
+  bool HasCellSet(const std::string& name) const { return this->GetCellSetIndex(name) >= 0; }
-  {
-    VTKM_ASSERT((index >= 0) && (index < this->GetNumberOfCellSets()));
-    return this->CellSets[static_cast<std::size_t>(index)];
-  }
 
   VTKM_CONT
-  bool HasCellSet(const std::string& name) const
+  const vtkm::cont::DynamicCellSet& GetCellSet(vtkm::Id index = 0) const;
-  {
-    bool found;
-    this->FindCellSetIndex(name, found);
-    return found;
-  }
 
+
+  /// Returns the index for the first cell set whose
+  /// name matches the provided string.
+  /// Will return -1 if no match is found
   VTKM_CONT
   vtkm::Id GetCellSetIndex(const std::string& name) const;
 
+
+  /// Returns the first DynamicCellSet that matches the provided name.
+  /// Will throw an exception if no match is found
   VTKM_CONT
-  vtkm::cont::DynamicCellSet GetCellSet(const std::string& name) const
+  const vtkm::cont::DynamicCellSet& GetCellSet(const std::string& name) const;
-  {
+
-    return this->GetCellSet(this->GetCellSetIndex(name));
-  }
 
   VTKM_CONT
   vtkm::IdComponent GetNumberOfCellSets() const
@@ -180,12 +185,6 @@ private:
   vtkm::Id FindFieldIndex(const std::string& name,
                           vtkm::cont::Field::Association association,
                           bool& found) const;
-
-  VTKM_CONT
-  vtkm::Id FindCoordinateSystemIndex(const std::string& name, bool& found) const;
-
-  VTKM_CONT
-  vtkm::Id FindCellSetIndex(const std::string& name, bool& found) const;
 };
 
 } // namespace cont