From b0c6e18ea70ad5803cc986afd2eabfb30f20bc97 Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Mon, 19 Aug 2019 09:04:53 -0400 Subject: [PATCH] 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 --- .../dataset-no-throw-on-index-queries.md | 11 +++ vtkm/cont/DataSet.cxx | 90 ++++++++++--------- vtkm/cont/DataSet.h | 61 +++++++------ 3 files changed, 90 insertions(+), 72 deletions(-) create mode 100644 docs/changelog/dataset-no-throw-on-index-queries.md diff --git a/docs/changelog/dataset-no-throw-on-index-queries.md b/docs/changelog/dataset-no-throw-on-index-queries.md new file mode 100644 index 000000000..3751ef1a9 --- /dev/null +++ b/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. + diff --git a/vtkm/cont/DataSet.cxx b/vtkm/cont/DataSet.cxx index 98c524299..bcde71f16 100644 --- a/vtkm/cont/DataSet.cxx +++ b/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 = this->FindCoordinateSystemIndex(name, found); - if (found) + vtkm::Id index = -1; + for (auto i = this->CoordSystems.begin(); i != this->CoordSystems.end(); ++i) { - return index; + if (i->GetName() == name) + { + index = static_cast(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(index)]; } vtkm::Id DataSet::GetCellSetIndex(const std::string& name) const { - bool found; - vtkm::Id index = this->FindCellSetIndex(name, found); - if (found) + vtkm::Id index = -1; + for (auto i = this->CellSets.begin(); i != this->CellSets.end(); ++i) { - return index; + if (i->GetName() == name) + { + index = static_cast(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(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(this->GetNumberOfCellSets()); ++index) - { - if (this->CellSets[index].GetName() == name) - { - found = true; - return static_cast(index); - } - } - found = false; - return -1; -} - } // namespace cont } // namespace vtkm diff --git a/vtkm/cont/DataSet.h b/vtkm/cont/DataSet.h index fd61cba0c..763dd0e43 100644 --- a/vtkm/cont/DataSet.h +++ b/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; - this->FindCoordinateSystemIndex(name, found); - return found; + return this->GetCoordinateSystemIndex(name) >= 0; } + 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 - { - return this->GetCoordinateSystem(this->GetCoordinateSystemIndex(name)); - } + const vtkm::cont::CoordinateSystem& GetCoordinateSystem(const std::string& name) const; 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 - { - VTKM_ASSERT((index >= 0) && (index < this->GetNumberOfCellSets())); - return this->CellSets[static_cast(index)]; - } + bool HasCellSet(const std::string& name) const { return this->GetCellSetIndex(name) >= 0; } VTKM_CONT - bool HasCellSet(const std::string& name) const - { - bool found; - this->FindCellSetIndex(name, found); - return found; - } + const vtkm::cont::DynamicCellSet& GetCellSet(vtkm::Id index = 0) const; + + /// 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 - { - return this->GetCellSet(this->GetCellSetIndex(name)); - } + const vtkm::cont::DynamicCellSet& GetCellSet(const std::string& name) const; + 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