From 5f14f4f0d62e49f181878c089fc5ba5ce9f81524 Mon Sep 17 00:00:00 2001 From: Oleg Alexandrov <oleg.alexandrov@gmail.com> Date: Mon, 2 May 2022 08:46:49 -0700 Subject: [PATCH] Avoid using pointers where a std::shared_ptr can be used (#384) * Avoid using pointers where a std::shared_ptr can be used * Use smart pointers in PluginTests.cpp * Use smart pointers in FrameCameraTests.cpp * Eliminate pointer deletion in Fixtures.h --- src/UsgsAstroFrameSensorModel.cpp | 7 +- src/UsgsAstroLsSensorModel.cpp | 13 +-- src/UsgsAstroPlugin.cpp | 13 +-- src/UsgsAstroPushFrameSensorModel.cpp | 7 +- src/UsgsAstroSarSensorModel.cpp | 53 +++++----- tests/Fixtures.h | 136 +++++++++++++------------- tests/FrameCameraTests.cpp | 92 +++++------------ tests/PluginTests.cpp | 64 +++++------- 8 files changed, 146 insertions(+), 239 deletions(-) diff --git a/src/UsgsAstroFrameSensorModel.cpp b/src/UsgsAstroFrameSensorModel.cpp index 7577371..52640a3 100644 --- a/src/UsgsAstroFrameSensorModel.cpp +++ b/src/UsgsAstroFrameSensorModel.cpp @@ -899,7 +899,7 @@ std::string UsgsAstroFrameSensorModel::constructStateFromIsd( MESSAGE_LOG("Constructing state from isd"); - csm::WarningList *parsingWarnings = new csm::WarningList; + std::shared_ptr<csm::WarningList> parsingWarnings(new csm::WarningList); state["m_modelName"] = ale::getSensorModelName(parsedIsd); state["m_imageIdentifier"] = ale::getImageId(parsedIsd); @@ -1115,8 +1115,6 @@ std::string UsgsAstroFrameSensorModel::constructStateFromIsd( warnings->insert(warnings->end(), parsingWarnings->begin(), parsingWarnings->end()); } - delete parsingWarnings; - parsingWarnings = nullptr; MESSAGE_LOG("ISD is invalid for creating the sensor model."); throw csm::Error(csm::Error::SENSOR_MODEL_NOT_CONSTRUCTIBLE, @@ -1124,9 +1122,6 @@ std::string UsgsAstroFrameSensorModel::constructStateFromIsd( "UsgsAstroFrameSensorModel::constructStateFromIsd"); } - delete parsingWarnings; - parsingWarnings = nullptr; - return state.dump(); } diff --git a/src/UsgsAstroLsSensorModel.cpp b/src/UsgsAstroLsSensorModel.cpp index fd03eed..2a4f91f 100644 --- a/src/UsgsAstroLsSensorModel.cpp +++ b/src/UsgsAstroLsSensorModel.cpp @@ -2265,7 +2265,7 @@ std::string UsgsAstroLsSensorModel::constructStateFromIsd( MESSAGE_LOG("Constructing state from Isd") // Instantiate UsgsAstroLineScanner sensor model json jsonIsd = json::parse(imageSupportData); - csm::WarningList* parsingWarnings = new csm::WarningList; + std::shared_ptr<csm::WarningList> parsingWarnings(new csm::WarningList); state["m_modelName"] = ale::getSensorModelName(jsonIsd); state["m_imageIdentifier"] = ale::getImageId(jsonIsd); @@ -2397,10 +2397,10 @@ std::string UsgsAstroLsSensorModel::constructStateFromIsd( std::vector<std::vector<double>> lineScanRate = ale::getLineScanRate(jsonIsd); state["m_intTimeLines"] = - getIntegrationStartLines(lineScanRate, parsingWarnings); + getIntegrationStartLines(lineScanRate, parsingWarnings.get()); state["m_intTimeStartTimes"] = - getIntegrationStartTimes(lineScanRate, parsingWarnings); - state["m_intTimes"] = getIntegrationTimes(lineScanRate, parsingWarnings); + getIntegrationStartTimes(lineScanRate, parsingWarnings.get()); + state["m_intTimes"] = getIntegrationTimes(lineScanRate, parsingWarnings.get()); MESSAGE_LOG( "m_intTimeLines: {} " "m_intTimeStartTimes: {} " @@ -2566,16 +2566,11 @@ std::string UsgsAstroLsSensorModel::constructStateFromIsd( warnings->insert(warnings->end(), parsingWarnings->begin(), parsingWarnings->end()); } - delete parsingWarnings; - parsingWarnings = nullptr; throw csm::Error(csm::Error::SENSOR_MODEL_NOT_CONSTRUCTIBLE, "ISD is invalid for creating the sensor model.", "UsgsAstroFrameSensorModel::constructStateFromIsd"); } - delete parsingWarnings; - parsingWarnings = nullptr; - // The state data will still be updated when a sensor model is created since // some state data is not in the ISD and requires a SM to compute them. return state.dump(); diff --git a/src/UsgsAstroPlugin.cpp b/src/UsgsAstroPlugin.cpp index 8b7968f..0b7b38f 100644 --- a/src/UsgsAstroPlugin.cpp +++ b/src/UsgsAstroPlugin.cpp @@ -165,12 +165,10 @@ bool UsgsAstroPlugin::canModelBeConstructedFromISD( const csm::Isd &imageSupportData, const std::string &modelName, csm::WarningList *warnings) const { try { - csm::Model *model = - constructModelFromISD(imageSupportData, modelName, warnings); - if (model) { - delete model; + std::shared_ptr<csm::Model> model(constructModelFromISD(imageSupportData, modelName, warnings)); + if (model) return true; - } + } catch (std::exception &e) { if (warnings) { std::string msg = "Could not create model ["; @@ -284,10 +282,9 @@ std::string UsgsAstroPlugin::convertISDToModelState( const csm::Isd &imageSupportData, const std::string &modelName, csm::WarningList *warnings) const { MESSAGE_LOG("Running convertISDToModelState"); - csm::Model *sensor_model = - constructModelFromISD(imageSupportData, modelName, warnings); + std::shared_ptr<csm::Model> sensor_model + (constructModelFromISD(imageSupportData, modelName, warnings)); std::string stateString = sensor_model->getModelState(); - delete sensor_model; return stateString; } diff --git a/src/UsgsAstroPushFrameSensorModel.cpp b/src/UsgsAstroPushFrameSensorModel.cpp index 5515db3..943cdee 100644 --- a/src/UsgsAstroPushFrameSensorModel.cpp +++ b/src/UsgsAstroPushFrameSensorModel.cpp @@ -2061,7 +2061,7 @@ std::string UsgsAstroPushFrameSensorModel::constructStateFromIsd( MESSAGE_LOG("Constructing state from Isd") // Instantiate UsgsAstroLineScanner sensor model json jsonIsd = json::parse(imageSupportData); - csm::WarningList* parsingWarnings = new csm::WarningList; + std::shared_ptr<csm::WarningList> parsingWarnings(new csm::WarningList); state["m_modelName"] = ale::getSensorModelName(jsonIsd); state["m_imageIdentifier"] = ale::getImageId(jsonIsd); @@ -2380,16 +2380,11 @@ std::string UsgsAstroPushFrameSensorModel::constructStateFromIsd( warnings->insert(warnings->end(), parsingWarnings->begin(), parsingWarnings->end()); } - delete parsingWarnings; - parsingWarnings = nullptr; throw csm::Error(csm::Error::SENSOR_MODEL_NOT_CONSTRUCTIBLE, "ISD is invalid for creating the sensor model.", "UsgsAstroFrameSensorModel::constructStateFromIsd"); } - delete parsingWarnings; - parsingWarnings = nullptr; - // The state data will still be updated when a sensor model is created since // some state data is not in the ISD and requires a SM to compute them. return state.dump(); diff --git a/src/UsgsAstroSarSensorModel.cpp b/src/UsgsAstroSarSensorModel.cpp index 291c45a..622a198 100644 --- a/src/UsgsAstroSarSensorModel.cpp +++ b/src/UsgsAstroSarSensorModel.cpp @@ -42,28 +42,28 @@ string UsgsAstroSarSensorModel::constructStateFromIsd( json isd = json::parse(imageSupportData); json state = {}; - csm::WarningList* parsingWarnings = new csm::WarningList; + std::shared_ptr<csm::WarningList> parsingWarnings(new csm::WarningList); - state["m_modelName"] = getSensorModelName(isd, parsingWarnings); - state["m_imageIdentifier"] = getImageId(isd, parsingWarnings); - state["m_sensorName"] = getSensorName(isd, parsingWarnings); - state["m_platformName"] = getPlatformName(isd, parsingWarnings); + state["m_modelName"] = getSensorModelName(isd, parsingWarnings.get()); + state["m_imageIdentifier"] = getImageId(isd, parsingWarnings.get()); + state["m_sensorName"] = getSensorName(isd, parsingWarnings.get()); + state["m_platformName"] = getPlatformName(isd, parsingWarnings.get()); - state["m_nLines"] = getTotalLines(isd, parsingWarnings); - state["m_nSamples"] = getTotalSamples(isd, parsingWarnings); + state["m_nLines"] = getTotalLines(isd, parsingWarnings.get()); + state["m_nSamples"] = getTotalSamples(isd, parsingWarnings.get()); // Zero computed state values state["m_referencePointXyz"] = vector<double>(3, 0.0); // sun_position and velocity are required for getIlluminationDirection - state["m_sunPosition"] = getSunPositions(isd, parsingWarnings); - state["m_sunVelocity"] = getSunVelocities(isd, parsingWarnings); + state["m_sunPosition"] = getSunPositions(isd, parsingWarnings.get()); + state["m_sunVelocity"] = getSunVelocities(isd, parsingWarnings.get()); - state["m_centerEphemerisTime"] = getCenterTime(isd, parsingWarnings); - state["m_startingEphemerisTime"] = getStartingTime(isd, parsingWarnings); - state["m_endingEphemerisTime"] = getEndingTime(isd, parsingWarnings); + state["m_centerEphemerisTime"] = getCenterTime(isd, parsingWarnings.get()); + state["m_startingEphemerisTime"] = getStartingTime(isd, parsingWarnings.get()); + state["m_endingEphemerisTime"] = getEndingTime(isd, parsingWarnings.get()); - state["m_exposureDuration"] = getExposureDuration(isd, parsingWarnings); + state["m_exposureDuration"] = getExposureDuration(isd, parsingWarnings.get()); try { state["m_dtEphem"] = isd.at("dt_ephemeris"); @@ -85,31 +85,31 @@ string UsgsAstroSarSensorModel::constructStateFromIsd( "UsgsAstroSarSensorModel::constructStateFromIsd()")); } - state["m_positions"] = getSensorPositions(isd, parsingWarnings); - state["m_velocities"] = getSensorVelocities(isd, parsingWarnings); + state["m_positions"] = getSensorPositions(isd, parsingWarnings.get()); + state["m_velocities"] = getSensorVelocities(isd, parsingWarnings.get()); state["m_currentParameterValue"] = vector<double>(NUM_PARAMETERS, 0.0); // get radii - state["m_minorAxis"] = getSemiMinorRadius(isd, parsingWarnings); - state["m_majorAxis"] = getSemiMajorRadius(isd, parsingWarnings); + state["m_minorAxis"] = getSemiMinorRadius(isd, parsingWarnings.get()); + state["m_majorAxis"] = getSemiMajorRadius(isd, parsingWarnings.get()); // set identifiers - state["m_platformIdentifier"] = getPlatformName(isd, parsingWarnings); - state["m_sensorIdentifier"] = getSensorName(isd, parsingWarnings); + state["m_platformIdentifier"] = getPlatformName(isd, parsingWarnings.get()); + state["m_sensorIdentifier"] = getSensorName(isd, parsingWarnings.get()); // get reference_height state["m_minElevation"] = -1000; state["m_maxElevation"] = 1000; // SAR specific values - state["m_scaledPixelWidth"] = getScaledPixelWidth(isd, parsingWarnings); + state["m_scaledPixelWidth"] = getScaledPixelWidth(isd, parsingWarnings.get()); state["m_scaleConversionCoefficients"] = - getScaleConversionCoefficients(isd, parsingWarnings); + getScaleConversionCoefficients(isd, parsingWarnings.get()); state["m_scaleConversionTimes"] = - getScaleConversionTimes(isd, parsingWarnings); - state["m_wavelength"] = getWavelength(isd, parsingWarnings); - state["m_lookDirection"] = getLookDirection(isd, parsingWarnings); + getScaleConversionTimes(isd, parsingWarnings.get()); + state["m_wavelength"] = getWavelength(isd, parsingWarnings.get()); + state["m_lookDirection"] = getLookDirection(isd, parsingWarnings.get()); // Default to identity covariance state["m_covariance"] = vector<double>(NUM_PARAMETERS * NUM_PARAMETERS, 0.0); @@ -127,16 +127,11 @@ string UsgsAstroSarSensorModel::constructStateFromIsd( csm::Warning warn = parsingWarnings->front(); message += warn.getMessage(); message += "]"; - parsingWarnings = nullptr; - delete parsingWarnings; MESSAGE_LOG(message); throw csm::Error(csm::Error::SENSOR_MODEL_NOT_CONSTRUCTIBLE, message, "UsgsAstroSarSensorModel::constructStateFromIsd"); } - delete parsingWarnings; - parsingWarnings = nullptr; - // The state data will still be updated when a sensor model is created since // some state data is not in the ISD and requires a SM to compute them. return state.dump(); diff --git a/tests/Fixtures.h b/tests/Fixtures.h index 91f81f9..8751032 100644 --- a/tests/Fixtures.h +++ b/tests/Fixtures.h @@ -40,6 +40,7 @@ inline void jsonToIsd(nlohmann::json &object, csm::Isd &isd, class FrameSensorModel : public ::testing::Test { protected: csm::Isd isd; + std::shared_ptr<csm::Model> model; UsgsAstroFrameSensorModel *sensorModel; void SetUp() override { @@ -48,24 +49,23 @@ class FrameSensorModel : public ::testing::Test { isd.setFilename("data/simpleFramerISD.img"); UsgsAstroPlugin frameCameraPlugin; - csm::Model *model = frameCameraPlugin.constructModelFromISD( - isd, UsgsAstroFrameSensorModel::_SENSOR_MODEL_NAME); - sensorModel = dynamic_cast<UsgsAstroFrameSensorModel *>(model); + model = std::shared_ptr<csm::Model>(frameCameraPlugin.constructModelFromISD( + isd, UsgsAstroFrameSensorModel::_SENSOR_MODEL_NAME)); + sensorModel = dynamic_cast<UsgsAstroFrameSensorModel *>(model.get()); ASSERT_NE(sensorModel, nullptr); } void TearDown() override { - if (sensorModel) { - delete sensorModel; - sensorModel = NULL; - } + // The object that sensorModel points to is managed by the smart pointer 'model'. + sensorModel = NULL; } }; class FrameSensorModelLogging : public ::testing::Test { protected: csm::Isd isd; + std::shared_ptr<csm::Model> model; UsgsAstroFrameSensorModel *sensorModel; std::ostringstream oss; @@ -75,9 +75,9 @@ class FrameSensorModelLogging : public ::testing::Test { isd.setFilename("data/simpleFramerISD.img"); UsgsAstroPlugin frameCameraPlugin; - csm::Model *model = frameCameraPlugin.constructModelFromISD( - isd, UsgsAstroFrameSensorModel::_SENSOR_MODEL_NAME); - sensorModel = dynamic_cast<UsgsAstroFrameSensorModel *>(model); + model = std::shared_ptr<csm::Model>(frameCameraPlugin.constructModelFromISD( + isd, UsgsAstroFrameSensorModel::_SENSOR_MODEL_NAME)); + sensorModel = dynamic_cast<UsgsAstroFrameSensorModel *>(model.get()); ASSERT_NE(sensorModel, nullptr); @@ -98,7 +98,7 @@ class FrameSensorModelLogging : public ::testing::Test { std::uintptr_t sensorId = reinterpret_cast<std::uintptr_t>(sensorModel); spdlog::drop(std::to_string(sensorId)); - delete sensorModel; + // No deletion is needed because the resource is owned by the smart pointer 'model'. sensorModel = NULL; } @@ -109,6 +109,7 @@ class FrameSensorModelLogging : public ::testing::Test { class OrbitalFrameSensorModel : public ::testing::Test { protected: csm::Isd isd; + std::shared_ptr<csm::Model> model; UsgsAstroFrameSensorModel *sensorModel; void SetUp() override { @@ -117,18 +118,16 @@ class OrbitalFrameSensorModel : public ::testing::Test { isd.setFilename("data/orbitalFramer.img"); UsgsAstroPlugin frameCameraPlugin; - csm::Model *model = frameCameraPlugin.constructModelFromISD( - isd, UsgsAstroFrameSensorModel::_SENSOR_MODEL_NAME); - sensorModel = dynamic_cast<UsgsAstroFrameSensorModel *>(model); + model = std::shared_ptr<csm::Model>(frameCameraPlugin.constructModelFromISD( + isd, UsgsAstroFrameSensorModel::_SENSOR_MODEL_NAME)); + sensorModel = dynamic_cast<UsgsAstroFrameSensorModel *>(model.get()); ASSERT_NE(sensorModel, nullptr); } void TearDown() override { - if (sensorModel) { - delete sensorModel; - sensorModel = NULL; - } + // No deletion is needed because the resource is owned by the smart pointer 'model'. + sensorModel = NULL; } }; @@ -153,6 +152,7 @@ class FramerParameterizedTest : public ::testing::TestWithParam<csm::ImageCoord> { protected: csm::Isd isd; + std::shared_ptr<csm::Model> model; std::string printIsd(csm::Isd &localIsd) { std::string str; @@ -166,11 +166,13 @@ class FramerParameterizedTest } UsgsAstroFrameSensorModel *createModel(csm::Isd &modifiedIsd) { UsgsAstroPlugin frameCameraPlugin; - csm::Model *model = frameCameraPlugin.constructModelFromISD( - modifiedIsd, UsgsAstroFrameSensorModel::_SENSOR_MODEL_NAME); + model = std::shared_ptr<csm::Model>(frameCameraPlugin.constructModelFromISD( + modifiedIsd, UsgsAstroFrameSensorModel::_SENSOR_MODEL_NAME)); + // The object to which sensorModel points is managed by the smart pointer in 'model'. + // That one will be deleted when this test is over. UsgsAstroFrameSensorModel *sensorModel = - dynamic_cast<UsgsAstroFrameSensorModel *>(model); + dynamic_cast<UsgsAstroFrameSensorModel *>(model.get()); if (sensorModel) return sensorModel; @@ -184,14 +186,16 @@ class FramerParameterizedTest class FrameStateTest : public ::testing::Test { protected: csm::Isd isd; + + // This function will return a pointer which is managed by the caller UsgsAstroFrameSensorModel *createModifiedStateSensorModel(std::string key, double newValue) { UsgsAstroPlugin cameraPlugin; csm::Model *model = cameraPlugin.constructModelFromISD( - isd, UsgsAstroFrameSensorModel::_SENSOR_MODEL_NAME); + isd, UsgsAstroFrameSensorModel::_SENSOR_MODEL_NAME); UsgsAstroFrameSensorModel *sensorModel = - dynamic_cast<UsgsAstroFrameSensorModel *>(model); + dynamic_cast<UsgsAstroFrameSensorModel *>(model); if (sensorModel) { sensorModel->getModelState(); std::string modelState = sensorModel->getModelState(); @@ -213,6 +217,7 @@ class FrameStateTest : public ::testing::Test { class ConstVelocityLineScanSensorModel : public ::testing::Test { protected: csm::Isd isd; + std::shared_ptr<csm::Model> model; UsgsAstroLsSensorModel *sensorModel; void SetUp() override { @@ -221,24 +226,24 @@ class ConstVelocityLineScanSensorModel : public ::testing::Test { isd.setFilename("data/constVelocityLineScan.img"); UsgsAstroPlugin cameraPlugin; - csm::Model *model = cameraPlugin.constructModelFromISD( - isd, UsgsAstroLsSensorModel::_SENSOR_MODEL_NAME); - sensorModel = dynamic_cast<UsgsAstroLsSensorModel *>(model); + model = std::shared_ptr<csm::Model>(cameraPlugin.constructModelFromISD( + isd, UsgsAstroLsSensorModel::_SENSOR_MODEL_NAME)); + sensorModel = dynamic_cast<UsgsAstroLsSensorModel *>(model.get()); ASSERT_NE(sensorModel, nullptr); } void TearDown() override { - if (sensorModel) { - delete sensorModel; + // No deletion is necessary since the resource is managed by the smart pointer in 'model' + if (sensorModel) sensorModel = NULL; - } } }; class OrbitalLineScanSensorModel : public ::testing::Test { protected: csm::Isd isd; + std::shared_ptr<csm::Model> model; UsgsAstroLsSensorModel *sensorModel; void SetUp() override { @@ -247,18 +252,16 @@ class OrbitalLineScanSensorModel : public ::testing::Test { isd.setFilename("data/orbitalLineScan.img"); UsgsAstroPlugin cameraPlugin; - csm::Model *model = cameraPlugin.constructModelFromISD( - isd, UsgsAstroLsSensorModel::_SENSOR_MODEL_NAME); - sensorModel = dynamic_cast<UsgsAstroLsSensorModel *>(model); - + model = std::shared_ptr<csm::Model>(cameraPlugin.constructModelFromISD( + isd, UsgsAstroLsSensorModel::_SENSOR_MODEL_NAME)); + sensorModel = dynamic_cast<UsgsAstroLsSensorModel *>(model.get()); + ASSERT_NE(sensorModel, nullptr); } void TearDown() override { - if (sensorModel) { - delete sensorModel; - sensorModel = NULL; - } + // The object that sensorModel points to is managed by the smart pointer 'model'. + sensorModel = NULL; } }; @@ -266,6 +269,7 @@ class FlippedOrbitalLineScanSensorModel : public ::testing::Test { protected: csm::Isd isd; UsgsAstroLsSensorModel *sensorModel; + std::shared_ptr<csm::Model> model; void SetUp() override { sensorModel = NULL; @@ -273,18 +277,16 @@ class FlippedOrbitalLineScanSensorModel : public ::testing::Test { isd.setFilename("data/flippedOrbitalLineScan.img"); UsgsAstroPlugin cameraPlugin; - csm::Model *model = cameraPlugin.constructModelFromISD( - isd, UsgsAstroLsSensorModel::_SENSOR_MODEL_NAME); - sensorModel = dynamic_cast<UsgsAstroLsSensorModel *>(model); + model = std::shared_ptr<csm::Model>(cameraPlugin.constructModelFromISD( + isd, UsgsAstroLsSensorModel::_SENSOR_MODEL_NAME)); + sensorModel = dynamic_cast<UsgsAstroLsSensorModel *>(model.get()); ASSERT_NE(sensorModel, nullptr); } void TearDown() override { - if (sensorModel) { - delete sensorModel; - sensorModel = NULL; - } + // The object that sensorModel points to is managed by the smart pointer 'model' + sensorModel = NULL; } }; @@ -293,6 +295,7 @@ class TwoLineScanSensorModels : public ::testing::Test { csm::Isd isd; UsgsAstroLsSensorModel *sensorModel1; UsgsAstroLsSensorModel *sensorModel2; + std::shared_ptr<csm::Model> model1, model2; void SetUp() override { sensorModel1 = nullptr; @@ -301,26 +304,21 @@ class TwoLineScanSensorModels : public ::testing::Test { isd.setFilename("data/orbitalLineScan.img"); UsgsAstroPlugin cameraPlugin; - csm::Model *model1 = cameraPlugin.constructModelFromISD( - isd, UsgsAstroLsSensorModel::_SENSOR_MODEL_NAME); - sensorModel1 = dynamic_cast<UsgsAstroLsSensorModel *>(model1); - csm::Model *model2 = cameraPlugin.constructModelFromISD( - isd, UsgsAstroLsSensorModel::_SENSOR_MODEL_NAME); - sensorModel2 = dynamic_cast<UsgsAstroLsSensorModel *>(model2); + model1 = std::shared_ptr<csm::Model>(cameraPlugin.constructModelFromISD( + isd, UsgsAstroLsSensorModel::_SENSOR_MODEL_NAME)); + sensorModel1 = dynamic_cast<UsgsAstroLsSensorModel *>(model1.get()); + model2 = std::shared_ptr<csm::Model>(cameraPlugin.constructModelFromISD( + isd, UsgsAstroLsSensorModel::_SENSOR_MODEL_NAME)); + sensorModel2 = dynamic_cast<UsgsAstroLsSensorModel *>(model2.get()); ASSERT_NE(sensorModel1, nullptr); ASSERT_NE(sensorModel2, nullptr); } void TearDown() override { - if (sensorModel1) { - delete sensorModel1; - sensorModel1 = nullptr; - } - if (sensorModel2) { - delete sensorModel2; - sensorModel2 = nullptr; - } + // Resource deallocation happens via the smart pointers + sensorModel1 = nullptr; + sensorModel2 = nullptr; } }; @@ -339,6 +337,7 @@ class SarSensorModel : public ::testing::Test { protected: csm::Isd isd; UsgsAstroSarSensorModel *sensorModel; + std::shared_ptr<csm::Model> model; void SetUp() override { sensorModel = NULL; @@ -346,17 +345,15 @@ class SarSensorModel : public ::testing::Test { isd.setFilename("data/orbitalSar.img"); UsgsAstroPlugin sarCameraPlugin; - csm::Model *model = sarCameraPlugin.constructModelFromISD( - isd, UsgsAstroSarSensorModel::_SENSOR_MODEL_NAME); - sensorModel = dynamic_cast<UsgsAstroSarSensorModel *>(model); + model = std::shared_ptr<csm::Model>(sarCameraPlugin.constructModelFromISD( + isd, UsgsAstroSarSensorModel::_SENSOR_MODEL_NAME)); + sensorModel = dynamic_cast<UsgsAstroSarSensorModel *>(model.get()); ASSERT_NE(sensorModel, nullptr); } void TearDown() override { - if (sensorModel) { - delete sensorModel; - sensorModel = NULL; - } + // The smart pointer will take care of dellocation + sensorModel = NULL; } }; @@ -372,11 +369,12 @@ class OrbitalPushFrameSensorModel : public ::testing::Test { void SetUp() override { isd.setFilename("data/orbitalPushFrame.img"); UsgsAstroPlugin cameraPlugin; + + csm::Model * model = cameraPlugin.constructModelFromISD( + isd, UsgsAstroPushFrameSensorModel::_SENSOR_MODEL_NAME); - csm::Model *model = cameraPlugin.constructModelFromISD( - isd, UsgsAstroPushFrameSensorModel::_SENSOR_MODEL_NAME); - sensorModel = std::shared_ptr<UsgsAstroPushFrameSensorModel>( - dynamic_cast<UsgsAstroPushFrameSensorModel *>(model)); + // sensorModel is a smart pointer so it will deallocate the data of 'model' + sensorModel = std::shared_ptr<UsgsAstroPushFrameSensorModel>(dynamic_cast<UsgsAstroPushFrameSensorModel *>(model)); ASSERT_NE(sensorModel, nullptr); } diff --git a/tests/FrameCameraTests.cpp b/tests/FrameCameraTests.cpp index 197c30c..663a406 100644 --- a/tests/FrameCameraTests.cpp +++ b/tests/FrameCameraTests.cpp @@ -184,8 +184,8 @@ TEST_F(OrbitalFrameSensorModel, ImageToProximateImagingLocus) { TEST_F(FrameStateTest, FL500_OffBody4) { std::string key = "m_focalLength"; double newValue = 500.0; - UsgsAstroFrameSensorModel* sensorModel = - createModifiedStateSensorModel(key, newValue); + std::shared_ptr<UsgsAstroFrameSensorModel> sensorModel( + createModifiedStateSensorModel(key, newValue)); ASSERT_NE(sensorModel, nullptr); csm::ImageCoord imagePt(15.0, 15.0); @@ -193,16 +193,13 @@ TEST_F(FrameStateTest, FL500_OffBody4) { EXPECT_NEAR(groundPt.x, 9.77688917, 1e-8); EXPECT_NEAR(groundPt.y, -1.48533467, 1e-8); EXPECT_NEAR(groundPt.z, -1.48533467, 1e-8); - - delete sensorModel; - sensorModel = NULL; } TEST_F(FrameStateTest, FL500_OffBody3) { std::string key = "m_focalLength"; double newValue = 500.0; - UsgsAstroFrameSensorModel* sensorModel = - createModifiedStateSensorModel(key, newValue); + std::shared_ptr<UsgsAstroFrameSensorModel> sensorModel( + createModifiedStateSensorModel(key, newValue)); ASSERT_NE(sensorModel, nullptr); csm::ImageCoord imagePt(0.0, 0.0); @@ -210,16 +207,13 @@ TEST_F(FrameStateTest, FL500_OffBody3) { EXPECT_NEAR(groundPt.x, 9.77688917, 1e-8); EXPECT_NEAR(groundPt.y, 1.48533467, 1e-8); EXPECT_NEAR(groundPt.z, 1.48533467, 1e-8); - - delete sensorModel; - sensorModel = NULL; } TEST_F(FrameStateTest, FL500_Center) { std::string key = "m_focalLength"; double newValue = 500.0; - UsgsAstroFrameSensorModel* sensorModel = - createModifiedStateSensorModel(key, newValue); + std::shared_ptr<UsgsAstroFrameSensorModel> sensorModel( + createModifiedStateSensorModel(key, newValue)); ASSERT_NE(sensorModel, nullptr); csm::ImageCoord imagePt(7.5, 7.5); @@ -227,16 +221,13 @@ TEST_F(FrameStateTest, FL500_Center) { EXPECT_NEAR(groundPt.x, 10.0, 1e-8); EXPECT_NEAR(groundPt.y, 0.0, 1e-8); EXPECT_NEAR(groundPt.z, 0.0, 1e-8); - - delete sensorModel; - sensorModel = NULL; } TEST_F(FrameStateTest, FL500_SlightlyOffCenter) { std::string key = "m_focalLength"; double newValue = 500.0; - UsgsAstroFrameSensorModel* sensorModel = - createModifiedStateSensorModel(key, newValue); + std::shared_ptr<UsgsAstroFrameSensorModel> sensorModel( + createModifiedStateSensorModel(key, newValue)); ASSERT_NE(sensorModel, nullptr); csm::ImageCoord imagePt(7.5, 6.5); @@ -244,9 +235,6 @@ TEST_F(FrameStateTest, FL500_SlightlyOffCenter) { EXPECT_NEAR(groundPt.x, 9.99803960, 1e-8); EXPECT_NEAR(groundPt.y, 0.0, 1e-8); EXPECT_NEAR(groundPt.z, 1.98000392e-01, 1e-8); - - delete sensorModel; - sensorModel = NULL; } // Ellipsoid axis tests: @@ -254,8 +242,8 @@ TEST_F(FrameStateTest, SemiMajorAxis100x_Center) { std::string key = "m_majorAxis"; double newValue = 1000.0; - UsgsAstroFrameSensorModel* sensorModel = - createModifiedStateSensorModel(key, newValue); + std::shared_ptr<UsgsAstroFrameSensorModel> sensorModel( + createModifiedStateSensorModel(key, newValue)); ASSERT_NE(sensorModel, nullptr); csm::ImageCoord imagePt(7.5, 7.5); @@ -263,16 +251,13 @@ TEST_F(FrameStateTest, SemiMajorAxis100x_Center) { EXPECT_NEAR(groundPt.x, 1000.0, 1e-8); EXPECT_NEAR(groundPt.y, 0.0, 1e-8); EXPECT_NEAR(groundPt.z, 0.0, 1e-8); - - delete sensorModel; - sensorModel = NULL; } TEST_F(FrameStateTest, SemiMajorAxis10x_SlightlyOffCenter) { std::string key = "m_majorAxis"; double newValue = 100.0; - UsgsAstroFrameSensorModel* sensorModel = - createModifiedStateSensorModel(key, newValue); + std::shared_ptr<UsgsAstroFrameSensorModel> sensorModel( + createModifiedStateSensorModel(key, newValue)); ASSERT_NE(sensorModel, nullptr); csm::ImageCoord imagePt(7.5, 6.5); @@ -283,9 +268,6 @@ TEST_F(FrameStateTest, SemiMajorAxis10x_SlightlyOffCenter) { EXPECT_NEAR(groundPt.x, 9.83606557e+01, 1e-7); EXPECT_NEAR(groundPt.y, 0.0, 1e-7); EXPECT_NEAR(groundPt.z, 1.80327869, 1e-7); - - delete sensorModel; - sensorModel = NULL; } // The following test is for the scenario where the semi_minor_axis is actually @@ -293,8 +275,8 @@ TEST_F(FrameStateTest, SemiMajorAxis10x_SlightlyOffCenter) { TEST_F(FrameStateTest, SemiMinorAxis10x_SlightlyOffCenter) { std::string key = "m_minorAxis"; double newValue = 100.0; - UsgsAstroFrameSensorModel* sensorModel = - createModifiedStateSensorModel(key, newValue); + std::shared_ptr<UsgsAstroFrameSensorModel> sensorModel( + createModifiedStateSensorModel(key, newValue)); ASSERT_NE(sensorModel, nullptr); csm::ImageCoord imagePt(7.5, 6.5); @@ -302,16 +284,13 @@ TEST_F(FrameStateTest, SemiMinorAxis10x_SlightlyOffCenter) { EXPECT_NEAR(groundPt.x, 9.99803960, 1e-8); EXPECT_NEAR(groundPt.y, 0.0, 1e-8); EXPECT_NEAR(groundPt.z, 1.98000392, 1e-8); - - delete sensorModel; - sensorModel = NULL; } TEST_F(FrameStateTest, SampleSumming) { std::string key = "m_detectorSampleSumming"; double newValue = 2.0; - UsgsAstroFrameSensorModel* sensorModel = - createModifiedStateSensorModel(key, newValue); + std::shared_ptr<UsgsAstroFrameSensorModel> sensorModel( + createModifiedStateSensorModel(key, newValue)); ASSERT_NE(sensorModel, nullptr); csm::ImageCoord imagePt(7.5, 3.75); @@ -319,16 +298,13 @@ TEST_F(FrameStateTest, SampleSumming) { EXPECT_NEAR(groundPt.x, 10.0, 1e-8); EXPECT_NEAR(groundPt.y, 0, 1e-8); EXPECT_NEAR(groundPt.z, 0, 1e-8); - - delete sensorModel; - sensorModel = NULL; } TEST_F(FrameStateTest, LineSumming) { std::string key = "m_detectorLineSumming"; double newValue = 2.0; - UsgsAstroFrameSensorModel* sensorModel = - createModifiedStateSensorModel(key, newValue); + std::shared_ptr<UsgsAstroFrameSensorModel> sensorModel( + createModifiedStateSensorModel(key, newValue)); ASSERT_NE(sensorModel, nullptr); csm::ImageCoord imagePt(3.75, 7.5); @@ -336,16 +312,13 @@ TEST_F(FrameStateTest, LineSumming) { EXPECT_NEAR(groundPt.x, 10.0, 1e-8); EXPECT_NEAR(groundPt.y, 0, 1e-8); EXPECT_NEAR(groundPt.z, 0, 1e-8); - - delete sensorModel; - sensorModel = NULL; } TEST_F(FrameStateTest, StartSample) { std::string key = "m_startingDetectorSample"; double newValue = 5.0; - UsgsAstroFrameSensorModel* sensorModel = - createModifiedStateSensorModel(key, newValue); + std::shared_ptr<UsgsAstroFrameSensorModel> sensorModel( + createModifiedStateSensorModel(key, newValue)); ASSERT_NE(sensorModel, nullptr); csm::ImageCoord imagePt(7.5, 2.5); @@ -353,16 +326,13 @@ TEST_F(FrameStateTest, StartSample) { EXPECT_NEAR(groundPt.x, 10.0, 1e-8); EXPECT_NEAR(groundPt.y, 0, 1e-8); EXPECT_NEAR(groundPt.z, 0, 1e-8); - - delete sensorModel; - sensorModel = NULL; } TEST_F(FrameStateTest, StartLine) { std::string key = "m_startingDetectorLine"; double newValue = 5.0; - UsgsAstroFrameSensorModel* sensorModel = - createModifiedStateSensorModel(key, newValue); + std::shared_ptr<UsgsAstroFrameSensorModel> sensorModel( + createModifiedStateSensorModel(key, newValue)); ASSERT_NE(sensorModel, nullptr); csm::ImageCoord imagePt(2.5, 7.5); @@ -370,9 +340,6 @@ TEST_F(FrameStateTest, StartLine) { EXPECT_NEAR(groundPt.x, 10.0, 1e-8); EXPECT_NEAR(groundPt.y, 0, 1e-8); EXPECT_NEAR(groundPt.z, 0, 1e-8); - - delete sensorModel; - sensorModel = NULL; } // **************************************************************************** @@ -390,9 +357,6 @@ TEST_F(FrameSensorModel, X10_SlightlyOffCenter) { EXPECT_NEAR(groundPt.x, 10.0, 1e-8); EXPECT_NEAR(groundPt.y, 0.0, 1e-8); EXPECT_NEAR(groundPt.z, 0.0, 1e-8); - - delete sensorModel; - sensorModel = NULL; } TEST_F(FrameSensorModel, X1e9_SlightlyOffCenter) { @@ -407,9 +371,6 @@ TEST_F(FrameSensorModel, X1e9_SlightlyOffCenter) { EXPECT_NEAR(groundPt.x, 3.99998400e+03, 1e-4); EXPECT_NEAR(groundPt.y, 0.0, 1e-4); EXPECT_NEAR(groundPt.z, 1.99999200e+06, 1e-4); - - delete sensorModel; - sensorModel = NULL; } // Angle rotations: @@ -430,9 +391,6 @@ TEST_F(FrameSensorModel, Rotation_omegaPi_Center) { EXPECT_NEAR(groundPt.x, -10.0, 1e-8); EXPECT_NEAR(groundPt.y, 0.0, 1e-8); EXPECT_NEAR(groundPt.z, 0.0, 1e-8); - - delete sensorModel; - sensorModel = NULL; } TEST_F(FrameSensorModel, Rotation_NPole_Center) { @@ -452,9 +410,6 @@ TEST_F(FrameSensorModel, Rotation_NPole_Center) { EXPECT_NEAR(groundPt.x, 0.0, 1e-8); EXPECT_NEAR(groundPt.y, 0.0, 1e-8); EXPECT_NEAR(groundPt.z, 10.0, 1e-8); - - delete sensorModel; - sensorModel = NULL; } TEST_F(FrameSensorModel, Rotation_SPole_Center) { @@ -469,9 +424,6 @@ TEST_F(FrameSensorModel, Rotation_SPole_Center) { EXPECT_NEAR(groundPt.x, 0.0, 1e-8); EXPECT_NEAR(groundPt.y, 0.0, 1e-8); EXPECT_NEAR(groundPt.z, -10.0, 1e-8); - - delete sensorModel; - sensorModel = NULL; } // **************************************************************************** diff --git a/tests/PluginTests.cpp b/tests/PluginTests.cpp index f4901f4..93acccf 100644 --- a/tests/PluginTests.cpp +++ b/tests/PluginTests.cpp @@ -93,33 +93,26 @@ TEST_F(FrameIsdTest, NotConstructible) { TEST_F(FrameIsdTest, ConstructValidCamera) { UsgsAstroPlugin testPlugin; - csm::Model *cameraModel = NULL; - EXPECT_NO_THROW(cameraModel = testPlugin.constructModelFromISD( - isd, "USGS_ASTRO_FRAME_SENSOR_MODEL", NULL)); + std::shared_ptr<csm::Model> cameraModel(NULL); + EXPECT_NO_THROW(cameraModel = std::shared_ptr<csm::Model>(testPlugin.constructModelFromISD + (isd, "USGS_ASTRO_FRAME_SENSOR_MODEL", NULL))); UsgsAstroFrameSensorModel *frameModel = - dynamic_cast<UsgsAstroFrameSensorModel *>(cameraModel); + dynamic_cast<UsgsAstroFrameSensorModel *>(cameraModel.get()); EXPECT_NE(frameModel, nullptr); - if (cameraModel) { - delete cameraModel; - } } -TEST_F(FrameIsdTest, ConstructInValidCamera) { +TEST_F(FrameIsdTest, ConstructInvalidCamera) { UsgsAstroPlugin testPlugin; isd.setFilename("data/empty.img"); - csm::Model *cameraModel = NULL; + std::shared_ptr<csm::Model> cameraModel(NULL); try { - testPlugin.constructModelFromISD(isd, "USGS_ASTRO_FRAME_SENSOR_MODEL", - nullptr); + cameraModel = std::shared_ptr<csm::Model>(testPlugin.constructModelFromISD(isd, "USGS_ASTRO_FRAME_SENSOR_MODEL", nullptr)); FAIL() << "Expected an error"; } catch (csm::Error &e) { EXPECT_EQ(e.getError(), csm::Error::SENSOR_MODEL_NOT_CONSTRUCTIBLE); } catch (...) { FAIL() << "Expected csm SENSOR_MODEL_NOT_CONSTRUCTIBLE error"; } - if (cameraModel) { - delete cameraModel; - } } TEST_F(ConstVelLineScanIsdTest, Constructible) { @@ -144,34 +137,28 @@ TEST_F(ConstVelLineScanIsdTest, NotConstructible) { TEST_F(ConstVelLineScanIsdTest, ConstructValidCamera) { UsgsAstroPlugin testPlugin; - csm::Model *cameraModel = NULL; - EXPECT_NO_THROW(cameraModel = testPlugin.constructModelFromISD( - isd, "USGS_ASTRO_LINE_SCANNER_SENSOR_MODEL", NULL)); + std::shared_ptr<csm::Model> cameraModel(NULL); + EXPECT_NO_THROW(cameraModel = std::shared_ptr<csm::Model>(testPlugin.constructModelFromISD + (isd, "USGS_ASTRO_LINE_SCANNER_SENSOR_MODEL", NULL))); UsgsAstroLsSensorModel *frameModel = - dynamic_cast<UsgsAstroLsSensorModel *>(cameraModel); + dynamic_cast<UsgsAstroLsSensorModel *>(cameraModel.get()); EXPECT_NE(frameModel, nullptr); - if (cameraModel) { - delete cameraModel; - } } -TEST_F(ConstVelLineScanIsdTest, ConstructInValidCamera) { +TEST_F(ConstVelLineScanIsdTest, ConstructInvalidCamera) { UsgsAstroPlugin testPlugin; isd.setFilename("data/empty.img"); csm::Model *cameraModel = NULL; try { - testPlugin.constructModelFromISD( - isd, "USGS_ASTRO_LINE_SCANNER_SENSOR_MODEL", nullptr); + std::shared_ptr<csm::Model> cameraModel(testPlugin.constructModelFromISD + (isd, "USGS_ASTRO_LINE_SCANNER_SENSOR_MODEL", nullptr)); FAIL() << "Expected an error"; - } catch (csm::Error &e) { EXPECT_EQ(e.getError(), csm::Error::SENSOR_MODEL_NOT_CONSTRUCTIBLE); } catch (...) { FAIL() << "Expected csm SENSOR_MODEL_NOT_CONSTRUCTIBLE error"; } - if (cameraModel) { - delete cameraModel; - } + } TEST_F(SarIsdTest, Constructible) { @@ -207,28 +194,24 @@ TEST_F(SarIsdTest, NotConstructible) { TEST_F(SarIsdTest, ConstructValidCamera) { UsgsAstroPlugin testPlugin; csm::WarningList warnings; - csm::Model *cameraModel = NULL; - EXPECT_NO_THROW(cameraModel = testPlugin.constructModelFromISD( - isd, "USGS_ASTRO_SAR_SENSOR_MODEL", &warnings)); + std::shared_ptr<csm::Model> cameraModel(NULL); + EXPECT_NO_THROW(cameraModel = std::shared_ptr<csm::Model>(testPlugin.constructModelFromISD + (isd, "USGS_ASTRO_SAR_SENSOR_MODEL", &warnings))); for (auto &warn : warnings) { std::cerr << "Warning in " << warn.getFunction() << std::endl; std::cerr << " " << warn.getMessage() << std::endl; } UsgsAstroSarSensorModel *sarModel = - dynamic_cast<UsgsAstroSarSensorModel *>(cameraModel); + dynamic_cast<UsgsAstroSarSensorModel *>(cameraModel.get()); EXPECT_NE(sarModel, nullptr); - if (cameraModel) { - delete cameraModel; - } } -TEST_F(SarIsdTest, ConstructInValidCamera) { +TEST_F(SarIsdTest, ConstructInvalidCamera) { UsgsAstroPlugin testPlugin; isd.setFilename("data/empty.img"); - csm::Model *cameraModel = NULL; + std::shared_ptr<csm::Model> cameraModel(NULL); try { - testPlugin.constructModelFromISD(isd, "USGS_ASTRO_SAR_SENSOR_MODEL", - nullptr); + cameraModel = std::shared_ptr<csm::Model>(testPlugin.constructModelFromISD(isd, "USGS_ASTRO_SAR_SENSOR_MODEL", nullptr)); FAIL() << "Expected an error"; } catch (csm::Error &e) { @@ -236,9 +219,6 @@ TEST_F(SarIsdTest, ConstructInValidCamera) { } catch (...) { FAIL() << "Expected csm SENSOR_MODEL_NOT_CONSTRUCTIBLE error"; } - if (cameraModel) { - delete cameraModel; - } } int main(int argc, char **argv) { -- GitLab