diff --git a/Changelog.md b/Changelog.md index ad610972b..3da13b8f0 100644 --- a/Changelog.md +++ b/Changelog.md @@ -32,12 +32,15 @@ 1. Defer regex construction to avoid static initialization. * [Pull request 289](https://github.com/ignitionrobotics/ign-math/pull/289) + +1. Defer Material::Predefined construction to avoid static initialization. + * [Pull request 290](https://github.com/ignitionrobotics/ign-math/pull/290) + +1. Resolve cppcheck errors by adding explicit constructors and consts. + * [Pull request 291](https://github.com/ignitionrobotics/ign-math/pull/291) 1. Remove virtual from destructors of copyable classes. * [Pull request 293](https://github.com/ignitionrobotics/ign-math/pull/293) - -1. Resolve cppcheck errors by adding explicit constructors and consts. - * [Pull request 291](https://github.com/ignitionrobotics/ign-math/pull/291) ## Ignition Math 6.x diff --git a/include/ignition/math/MaterialType.hh b/include/ignition/math/MaterialType.hh index eae22f3a4..72f36496f 100644 --- a/include/ignition/math/MaterialType.hh +++ b/include/ignition/math/MaterialType.hh @@ -14,8 +14,8 @@ * limitations under the License. * */ -#ifndef IGNITION_MATH_MATERIALTYPES_HH_ -#define IGNITION_MATH_MATERIALTYPES_HH_ +#ifndef IGNITION_MATH_MATERIALTYPE_HH_ +#define IGNITION_MATH_MATERIALTYPE_HH_ #include #include diff --git a/src/Material.cc b/src/Material.cc index 931accbbe..0c6647fa2 100644 --- a/src/Material.cc +++ b/src/Material.cc @@ -16,6 +16,10 @@ */ #include "ignition/math/Material.hh" + +#include +#include + #include "ignition/math/Helpers.hh" // Placing the kMaterialData in a separate file for conveniece and clarity. @@ -24,24 +28,17 @@ using namespace ignition; using namespace math; -// Initialize the static map of Material objects based on the kMaterialData. -static const std::map kMaterials = []() +// Private data for the Material class +class ignition::math::MaterialPrivate { - std::map matMap; - - for (const std::pair &mat : kMaterialData) + /// \brief Set from a kMaterialData constant. + public: void SetFrom(const std::pair& _input) { - matMap[mat.first].SetType(mat.first); - matMap[mat.first].SetName(mat.second.name); - matMap[mat.first].SetDensity(mat.second.density); + type = _input.first; + name = _input.second.name; + density = _input.second.density; } - return matMap; -}(); - -// Private data for the Material class -class ignition::math::MaterialPrivate -{ /// \brief The material type. public: MaterialType type = MaterialType::UNKNOWN_MATERIAL; @@ -63,11 +60,13 @@ Material::Material() Material::Material(const MaterialType _type) : dataPtr(new MaterialPrivate) { - if (kMaterials.find(_type) != kMaterials.end()) + auto iter = std::find_if(std::begin(kMaterialData), std::end(kMaterialData), + [_type](const auto& item) { + return item.first == _type; + }); + if (iter != std::end(kMaterialData)) { - this->dataPtr->type = _type; - this->dataPtr->name = kMaterials.at(_type).Name(); - this->dataPtr->density = kMaterials.at(_type).Density(); + this->dataPtr->SetFrom(*iter); } } @@ -79,12 +78,13 @@ Material::Material(const std::string &_typename) std::string material = _typename; std::transform(material.begin(), material.end(), material.begin(), ::tolower); - for (const std::pair &mat : kMaterials) + auto iter = std::find_if(std::begin(kMaterialData), std::end(kMaterialData), + [&material](const auto& item) { + return item.second.name == material; + }); + if (iter != std::end(kMaterialData)) { - if (mat.second.Name() == material) - { - *this = mat.second; - } + this->dataPtr->SetFrom(*iter); } } @@ -121,7 +121,24 @@ Material::~Material() /////////////////////////////// const std::map &Material::Predefined() { - return kMaterials; + using Map = std::map; + + // Initialize the static map of Material objects based on the kMaterialData. + // We construct upon first use and never destroy it, in order to avoid the + // [Static Initialization Order Fiasco](https://en.cppreference.com/w/cpp/language/siof). + static const Map * const kMaterials = []() + { + auto temporary = std::make_unique(); + for (const auto &item : kMaterialData) + { + MaterialType type = item.first; + Material& material = (*temporary)[type]; + material.dataPtr->SetFrom(item); + } + return temporary.release(); + }(); + + return *kMaterials; } /////////////////////////////// @@ -198,13 +215,13 @@ void Material::SetToNearestDensity(const double _value, const double _epsilon) double min = MAX_D; Material result; - for (const std::pair &mat : kMaterials) + for (const auto &item : kMaterialData) { - double diff = std::fabs(mat.second.Density() - _value); + double diff = std::fabs(item.second.density - _value); if (diff < min && diff < _epsilon) { min = diff; - result = mat.second; + result.dataPtr->SetFrom(item); } } diff --git a/src/MaterialType.hh b/src/MaterialType.hh index cda91a23e..aba2eb6fa 100644 --- a/src/MaterialType.hh +++ b/src/MaterialType.hh @@ -14,11 +14,11 @@ * limitations under the License. * */ -#ifndef IGNITION_MATERIAL_HH_ -#define IGNITION_MATERIAL_HH_ +#ifndef IGNITION_MATH_SRC_MATERIAL_TYPE_HH_ +#define IGNITION_MATH_SRC_MATERIAL_TYPE_HH_ -#include -#include +#include +#include using namespace ignition; using namespace math; @@ -27,17 +27,19 @@ using namespace math; struct MaterialData { // Name of the material - std::string name; + // cppcheck-suppress unusedStructMember + const char * const name; // Density of the material // cppcheck-suppress unusedStructMember - double density; + const double density; }; // The mapping of material type to name and density values. // If you modify this map, make sure to also modify the MaterialType enum in -// include/ignition/math/MaterialTypes.hh -static std::map kMaterialData = +// include/ignition/math/MaterialTypes.hh. The compiler will also complain if +// you forget to change the std::array<..., N> compile-time size constant. +constexpr std::array, 13> kMaterialData = {{ {MaterialType::STYROFOAM, {"styrofoam", 75.0}}, {MaterialType::PINE, {"pine", 373.0}},