Some work on the Decoder and ImageParser to make the ownership of data more clear

modules/newhorizons/util/decoder.cpp

@@ -23,8 +23,11 @@
  ****************************************************************************************/
 
 #include <modules/newhorizons/util/decoder.h>
+
 #include <openspace/util/factorymanager.h>
+
 #include <ghoul/logging/logmanager.h>
+#include <ghoul/misc/exception.h>
 
 namespace {
 const std::string _loggerCat = "Decoder";
@@ -32,29 +35,22 @@ const std::string _loggerCat = "Decoder";
 
 namespace openspace {
 
-Decoder* Decoder::createFromDictionary(const ghoul::Dictionary& dictionary, const std::string& type)
+std::unique_ptr<Decoder> Decoder::createFromDictionary(
+    const ghoul::Dictionary& dictionary, const std::string& type)
 {
     ghoul::TemplateFactory<Decoder>* factory
         = FactoryManager::ref().factory<Decoder>();
     Decoder* result = factory->create(type, dictionary);
 
     if (result == nullptr) {
-        LERROR("Failed creating Payload object of type '" << type << "'");
-        return nullptr;
+        throw ghoul::RuntimeError(
+            "Failed creating payload object of type '" + type + '"',
+            "Decoder"
+        );
     }
-    return result;
+    return std::unique_ptr<Decoder>(result);
 }
 
-Decoder::Decoder()
-{
-}
-    
-Decoder::Decoder(const ghoul::Dictionary& dictionary)
-{
-}
-    
-Decoder::~Decoder()
-{
-}
+Decoder::~Decoder() {}
     
 } // namespace openspace

modules/newhorizons/util/decoder.h

@@ -26,20 +26,23 @@
 #define __DECODER_H__
 
 #include <ghoul/misc/dictionary.h>
-#include <openspace/util/updatestructures.h>
+
+#include <memory>
 
 namespace openspace {
 
 class Decoder {
 public:
-    static Decoder* createFromDictionary(const ghoul::Dictionary& dictionary, const std::string& type);
+    static std::unique_ptr<Decoder> createFromDictionary(
+        const ghoul::Dictionary& dictionary, const std::string& type);
 
-    Decoder(const ghoul::Dictionary& dictionary);
     virtual ~Decoder();
+
     virtual std::string getDecoderType() = 0;
     virtual std::vector<std::string> getTranslation() = 0;
+
 protected:
-    Decoder();
+    Decoder() = default;
 };
 
 }  // namespace openspace

modules/newhorizons/util/hongkangparser.cpp

@@ -75,10 +75,10 @@ HongKangParser::HongKangParser(std::string name, std::string fileName,
                 ghoul::Dictionary decoderDictionary;
                 translationDictionary.getValue(currentKey, decoderDictionary);
 
-                Decoder* decoder = Decoder::createFromDictionary(decoderDictionary, decoders[i]);
+                auto decoder = Decoder::createFromDictionary(decoderDictionary, decoders[i]);
                 //insert decoder to map - this will be used in the parser to determine
                 //behavioral characteristics of each instrument
-                _fileTranslation[keys[j]] = decoder;
+                _fileTranslation[keys[j]] = std::move(decoder);
             }
         }
         //Hong's playbook needs _only_ instrument translation though.
@@ -201,7 +201,7 @@ bool HongKangParser::create() {
                         if (it->second->getDecoderType() == "SCANNER"){ // SCANNER START 
                             scan_start = time;
 
-                            InstrumentDecoder* scanner = static_cast<InstrumentDecoder*>(it->second);
+                            InstrumentDecoder* scanner = static_cast<InstrumentDecoder*>(it->second.get());
                             std::string endNominal = scanner->getStopCommand();
 
                             // store current position in file

modules/newhorizons/util/hongkangparser.h

@@ -46,7 +46,6 @@ public:
 
     bool create() override;
     void findPlaybookSpecifiedTarget(std::string line, std::string& target);
-    virtual std::map<std::string, Decoder*> getTranslation(){ return _fileTranslation; };
 
 private:
     double getMetFromET(double et);
@@ -70,7 +69,7 @@ private:
     std::string _name;
     std::string _fileName;
     std::string _spacecraft;
-    std::map<std::string, Decoder*> _fileTranslation;
+    std::map<std::string, std::unique_ptr<Decoder>> _fileTranslation;
     std::vector<std::string> _potentialTargets;
 };
 

modules/newhorizons/util/imagesequencer.cpp

@@ -338,7 +338,7 @@ void ImageSequencer::runSequenceParser(SequenceParser* parser){
     bool parserComplete = parser->create();
     if (parserComplete){
         // get new data 
-        std::map<std::string, Decoder*> translations = parser->getTranslation(); // in1
+        std::map<std::string, std::unique_ptr<Decoder>>& translations = parser->getTranslation(); // in1
         std::map<std::string, ImageSubset> imageData = parser->getSubsetMap();   // in2
         std::vector<std::pair<std::string, TimeRange>> instrumentTimes = parser->getIstrumentTimes(); //in3
         std::vector<std::pair<double, std::string>> targetTimes = parser->getTargetTimes();  //in4
@@ -349,11 +349,14 @@ void ImageSequencer::runSequenceParser(SequenceParser* parser){
             LERROR("Missing sequence data");
             return;
         }
-            
+         
 
         // append data
-        _fileTranslation.insert(translations.begin(), translations.end());
-        for (auto it : imageData){
+        for (auto& it : translations) {
+            _fileTranslation[it.first] = std::move(it.second);
+        }
+
+        for (auto& it : imageData){
             if (_subsetMap.find(it.first) == _subsetMap.end()) {
                 // if key not exist yet - add sequence data for key (target)
                 _subsetMap.insert(it);
@@ -404,7 +407,7 @@ void ImageSequencer::runSequenceParser(SequenceParser* parser){
         sortData();
 
         // extract payload from _fileTranslation 
-        for (auto t : _fileTranslation){
+        for (auto& t : _fileTranslation){
             if (t.second->getDecoderType() == "CAMERA" ||
                 t.second->getDecoderType() == "SCANNER"){
                 std::vector<std::string> spiceIDs = t.second->getTranslation();

modules/newhorizons/util/imagesequencer.h

@@ -152,7 +152,7 @@ private:
      * \see Decoder
      * \see (projection mod files)
      */
-    std::map<std::string, Decoder*> _fileTranslation;
+    std::map<std::string, std::unique_ptr<Decoder>> _fileTranslation;
 
     /*
      * This is the main container of image data. The key is the target name, 

modules/newhorizons/util/instrumenttimesparser.cpp

@@ -65,7 +65,7 @@ InstrumentTimesParser::InstrumentTimesParser(
     for (const auto& instrumentKey : instruments.keys()) {
         ghoul::Dictionary instrument = instruments.value<ghoul::Dictionary>(instrumentKey);
         ghoul::Dictionary files = instrument.value<ghoul::Dictionary>(KeyInstrumentFiles);
-        _fileTranslation[instrumentKey] = Decoder::createFromDictionary(instrument, KeyInstrument);
+        _fileTranslation[instrumentKey] = std::move(Decoder::createFromDictionary(instrument, KeyInstrument));
         for (int i = 0; i < files.size(); i++) {
             std::string filename = files.value<std::string>(std::to_string(i + 1));
             _instrumentFiles[instrumentKey].push_back(filename);

modules/newhorizons/util/instrumenttimesparser.h

@@ -45,13 +45,7 @@ public:
 
     bool create() override;
 
-    //virtual std::map<std::string, Decoder*> getTranslation() override;
-
-    // temporary need to figure this out
-   std::map<std::string, Decoder*> getTranslation(){ return _fileTranslation; };
-
 private:
-    
     std::regex _pattern;
 
     std::map<std::string, std::vector<std::string>> _instrumentFiles;
@@ -59,7 +53,7 @@ private:
     std::string _name;
     std::string _fileName;
     std::string _spacecraft;
-    std::map<std::string, Decoder*> _fileTranslation;
+    std::map<std::string, std::unique_ptr<Decoder>> _fileTranslation;
     std::vector<std::string> _specsOfInterest;
 
     std::string _target;

modules/newhorizons/util/labelparser.cpp

@@ -22,18 +22,22 @@
  * OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                                         *
  ****************************************************************************************/
 
-#include <ghoul/logging/logmanager.h>
-#include <ghoul/filesystem/filesystem.h>
-#include <ghoul/filesystem/directory.h>
-#include <openspace/util/time.h>
-#include <openspace/util/spicemanager.h>
+#include <modules/newhorizons/util/labelparser.h>
+
 #include <modules/newhorizons/util/decoder.h>
+
+#include <openspace/util/spicemanager.h>
+#include <openspace/util/time.h>
+
+#include <ghoul/filesystem/directory.h>
+#include <ghoul/filesystem/filesystem.h>
+#include <ghoul/logging/logmanager.h>
+
 #include <fstream>
 #include <iterator>
 #include <iomanip>
 #include <limits>
 
-#include <modules/newhorizons/util/labelparser.h>
 
 namespace {
     const std::string _loggerCat = "LabelParser";
@@ -54,24 +58,24 @@ LabelParser::LabelParser(std::string name, std::string fileName,
     //get the different instrument types
     const std::vector<std::string>& decoders = translationDictionary.keys();
     //for each decoder (assuming might have more if hong makes changes)
-    for (int i = 0; i < decoders.size(); i++){
+    for (int i = 0; i < decoders.size(); ++i) {
         ghoul::Dictionary typeDictionary;
         translationDictionary.getValue(decoders[i], typeDictionary);
 
         //create dictionary containing all {playbookKeys , spice IDs}
-        if (decoders[i] == "Instrument"){
+        if (decoders[i] == "Instrument") {
             //for each playbook call -> create a Decoder object
-            const std::vector<std::string>& keys = typeDictionary.keys();
-            for (int j = 0; j < keys.size(); j++){
+            std::vector<std::string> keys = typeDictionary.keys();
+            for (int j = 0; j < keys.size(); ++j){
                 std::string currentKey = decoders[i] + "." + keys[j];
 
-                ghoul::Dictionary decoderDictionary;
-                translationDictionary.getValue(currentKey, decoderDictionary);
+                ghoul::Dictionary decoderDictionary =
+                    translationDictionary.value<ghoul::Dictionary>(currentKey);
 
-                Decoder *decoder = Decoder::createFromDictionary(decoderDictionary, decoders[i]);
+                auto decoder = Decoder::createFromDictionary(decoderDictionary, decoders[i]);
                 //insert decoder to map - this will be used in the parser to determine
                 //behavioral characteristics of each instrument
-                _fileTranslation[keys[j]] = decoder;
+                _fileTranslation[keys[j]] = std::move(decoder);
             }
         }
         if (decoders[i] == "Target"){
@@ -91,17 +95,17 @@ LabelParser::LabelParser(std::string name, std::string fileName,
             for (int j = 0; j < keys.size(); j++){
                 ghoul::Dictionary itemDictionary;
                 convertDictionary.getValue(keys[j], itemDictionary);
-                Decoder *decoder = Decoder::createFromDictionary(itemDictionary, decoders[i]);
+                auto decoder = Decoder::createFromDictionary(itemDictionary, decoders[i]);
                 //insert decoder to map - this will be used in the parser to determine
                 //behavioral characteristics of each instrument
-                _fileTranslation[keys[j]] = decoder;
+                _fileTranslation[keys[j]] = std::move(decoder);
             };
         }
     }
 }
 
 std::string LabelParser::decode(std::string line){
-    for (auto key : _fileTranslation){
+    for (auto& key : _fileTranslation){
         std::size_t value = line.find(key.first);
         if (value != std::string::npos){
             std::string toTranslate = line.substr(value);
@@ -121,7 +125,7 @@ std::string LabelParser::decode(std::string line){
 }
 
 std::string LabelParser::encode(std::string line) {
-    for (auto key : _fileTranslation) {
+    for (auto& key : _fileTranslation) {
         std::size_t value = line.find(key.first);
         if (value != std::string::npos) {
             return line.substr(value);

modules/newhorizons/util/labelparser.h

@@ -43,7 +43,7 @@ public:
     bool create() override;
 
     // temporary need to figure this out
-    std::map<std::string, Decoder*> getTranslation(){ return _fileTranslation; };
+    //std::map<std::string, Decoder*> getTranslation() { return _fileTranslation; };
 
 private:
     void createImage(Image& image,
@@ -64,7 +64,6 @@ private:
     std::string _name;
     std::string _fileName;
     std::string _spacecraft;
-    std::map<std::string, Decoder*> _fileTranslation;
     std::vector<std::string> _specsOfInterest;
 
     std::string _target;

modules/newhorizons/util/sequenceparser.cpp

@@ -52,6 +52,9 @@ std::vector<double> SequenceParser::getCaptureProgression(){
     return _captureProgression;
 };
 
+std::map<std::string, std::unique_ptr<Decoder>>& SequenceParser::getTranslation() {
+    return _fileTranslation;
+}
 
 template <typename T>
 void writeToBuffer(std::vector<char>& buffer, size_t& currentWriteLocation, T value) {

modules/newhorizons/util/sequenceparser.h

@@ -28,14 +28,15 @@
 #include <openspace/network/networkengine.h>
 #include <openspace/util/timerange.h>
 
+#include <modules/newhorizons/util/decoder.h>
+
 #include <map>
+#include <memory>
 #include <string>
 #include <vector>
 
 namespace openspace {
 
-class Decoder;
-
 struct Image {
     TimeRange timeRange;
     std::string path;
@@ -56,7 +57,7 @@ public:
     virtual std::map<std::string, ImageSubset> getSubsetMap() final;
     virtual std::vector<std::pair<std::string, TimeRange>> getIstrumentTimes() final;
     virtual std::vector<std::pair<double, std::string>> getTargetTimes() final;
-    virtual std::map<std::string, Decoder*> getTranslation() = 0;
+    std::map<std::string, std::unique_ptr<Decoder>>& getTranslation();
     virtual std::vector<double> getCaptureProgression() final;
 
 protected:
@@ -67,6 +68,8 @@ protected:
     std::vector<std::pair<double, std::string>> _targetTimes;
     std::vector<double> _captureProgression;
 
+    std::map<std::string, std::unique_ptr<Decoder>> _fileTranslation;
+
     NetworkEngine::MessageIdentifier _messageIdentifier;
 };