Merge pull request #16 from dezed-ukaea/main

Some smells

include/fdp/exceptions.hxx

@@ -7,36 +7,36 @@
 namespace FDP {
 class config_parsing_error : public std::runtime_error {
 public:
-  config_parsing_error(const std::string message)
+  config_parsing_error(const std::string& message)
       : std::runtime_error(message) {}
 };
 
 class rest_apiquery_error : public std::runtime_error {
 public:
-  rest_apiquery_error(const std::string message)
+  rest_apiquery_error(const std::string& message)
       : std::runtime_error(message) {}
 };
 
 class json_parse_error : public std::runtime_error {
 public:
-  json_parse_error(const std::string message) : std::runtime_error(message) {}
+  json_parse_error(const std::string& message) : std::runtime_error(message) {}
 };
 
 class validation_error : public std::runtime_error {
 public:
-  validation_error(const std::string message) : std::runtime_error(message) {}
+  validation_error(const std::string& message) : std::runtime_error(message) {}
 };
 
 class sync_error : public std::runtime_error {
 public:
-  sync_error(const std::string message) : std::runtime_error(message) {}
+  sync_error(const std::string& message) : std::runtime_error(message) {}
 };
 
 class write_error : public std::runtime_error {
 public:
-  write_error(const std::string message) : std::runtime_error(message) {}
+  write_error(const std::string& message) : std::runtime_error(message) {}
 };
 
 }; // namespace FDP
 
-#endif
+#endif

include/fdp/objects/api_object.hxx

@@ -16,6 +16,7 @@ namespace FDP {
         private:
             Json::Value obj_;
 
+        protected:
             ApiObject();
 
         public:
@@ -71,15 +72,15 @@ namespace FDP {
              * @param key 
              * @return std::string 
              */
-            std::string get_value_as_string(std::string key) const;
+            std::string get_value_as_string( const std::string& key) const;
 
             /**
              * @brief Get the value of a given key as int object
              * 
              * @param key 
              * @return int 
              */
-            int get_value_as_int(std::string key) const;
+            int get_value_as_int( const std::string& key) const;
 
             /**
              * @brief Get the first component of the object
@@ -96,6 +97,6 @@ namespace FDP {
              */
             bool is_empty();
     };
-};
 
+}
 #endif

include/fdp/objects/config.hxx

@@ -214,7 +214,7 @@ namespace FDP {
             /**
              * @brief Get the code run uuid
              * 
-             * @return std::string 
+             * @return const std::string&
              */
             std::string get_code_run_uuid() const;
 
@@ -263,14 +263,14 @@ namespace FDP {
              * 
              * @return std::shared_ptr<API> 
              */
-             API::sptr get_api(){return api_;}
+             API::sptr get_api() const {return api_;}
 
             /**
              * @brief Get the rest api location (local / remote)
              * 
              * @return RESTAPI 
              */
-            RESTAPI get_rest_api_location(){return rest_api_location_;}
+            RESTAPI get_rest_api_location() const {return rest_api_location_;}
 
     };
 };

include/fdp/objects/io_object.hxx

@@ -33,7 +33,7 @@ namespace FDP {
              * @brief Construct a new IOObject object (empty constructor)
              * 
              */
-            IOObject(){};
+            IOObject() = default;
             /**
              * @brief Construct a new IOObject object
              * Used to store inputs and outputs for in the config
@@ -46,12 +46,12 @@ namespace FDP {
              * @param data_product_description data product description
              * @param isPublic whether or not the data product should be public
              */
-            IOObject(std::string data_product,
-                    std::string use_data_product,
-                    std::string use_version,
-                    std::string use_namespace,
+            IOObject (const std::string& data_product,
+                    const std::string& use_data_product,
+                    const std::string& use_version,
+                    const std::string& use_namespace,
                     ghc::filesystem::path path,
-                    std::string data_product_description,
+                    const std::string& data_product_description,
                     bool isPublic)
                 :
                     data_product_(data_product),
@@ -74,10 +74,10 @@ namespace FDP {
              * @param component_obj the data_product component object as a shared pointer
              * @param data_product_obj the data_product object as a shared pointer
              */
-            IOObject(std::string data_product,
-                        std::string use_data_product,
-                        std::string use_version,
-                        std::string use_namespace,
+            IOObject( const std::string& data_product,
+                       const  std::string& use_data_product,
+                       const  std::string& use_version,
+                        const std::string& use_namespace,
                         ghc::filesystem::path path,
                         ApiObject &component_obj,
                         ApiObject &data_product_obj) 
@@ -93,81 +93,81 @@ namespace FDP {
             /**
              * @brief Get the data product as a string
              * 
-             * @return std::string 
+             * @return const std::string&
              */
-            std::string get_data_product() {return data_product_;}
+            const std::string& get_data_product() const {return data_product_;}
 
             /**
              * @brief Get the use data product  as a string
              * 
-             * @return std::string 
+             * @return const  std::string&
              */
-            std::string get_use_data_product() {return use_data_product_;}
+            const std::string& get_use_data_product() const {return use_data_product_;}
 
             /**
              * @brief Get the use component as a string
              * 
-             * @return std::string 
+             * @return const std::string&
              */
-            std::string get_use_component() {return use_component_;}
+            const std::string& get_use_component() const {return use_component_;}
 
             /**
              * @brief Get the use version  as a string
              * 
-             * @return std::string 
+             * @return const std::string&
              */
-            std::string get_use_version() {return use_version_;}
+            const std::string& get_use_version() const {return use_version_;}
 
             /**
              * @brief Get the use namespace as a string
              * 
-             * @return std::string 
+             * @return const std::string& 
              */
-            std::string get_use_namespace() {return use_namespace_;}
+            const std::string& get_use_namespace() {return use_namespace_;}
 
             /**
              * @brief Get the path of the data product
              * 
              * @return ghc::filesystem::path 
              */
-            ghc::filesystem::path get_path() {return path_;}
+            ghc::filesystem::path get_path() const {return path_;}
 
             /**
              * @brief Get the data product description as a string
              * 
              * @return std::string 
              */
-            std::string get_data_product_description() {return data_product_description_;}
+            const std::string& get_data_product_description() const {return data_product_description_;}
 
             /**
              * @brief Get the component description as a string
              * 
              * @return std::string 
              */
-            std::string get_component_description() {return component_description_;}
+            const std::string& get_component_description() {return component_description_;}
 
             /**
              * @brief Check whether the data product is public
              * 
              * @return true the data product is public
              * @return false the data product is not public
              */
-            bool is_public() {return public_;}
+            bool is_public() const {return public_;}
 
             /**
              * @brief Get the component object object
              * 
              * @return std::shared_ptr<ApiObject> 
              */
-             ApiObject::sptr get_component_object() {return component_obj_;}
+             ApiObject::sptr get_component_object() const {return component_obj_;}
 
 
             /**
              * @brief Get the data product object object
              * 
              * @return std::shared_ptr<ApiObject> 
              */
-             ApiObject::sptr get_data_product_object() {return data_product_obj_;}
+             ApiObject::sptr get_data_product_object() const {return data_product_obj_;}
 
             /**
              * @brief Set the component object object

include/fdp/utilities/logging.hxx

@@ -15,20 +15,22 @@
 #include "spdlog/spdlog.h"
 
 namespace FDP {
-/*! **************************************************************************
- * @brief Create a the global logger pointer
- * @author K. Zarebski (UKAEA)
- *
- * @return a shared pointer to the global logger instance
- ****************************************************************************/
-std::shared_ptr<spdlog::logger> create_logger_();
 
-/**
- * @brief Global logger used across all FDP scripts
- * @author K. Zarebski (UKAEA)
- *
- */
-const std::shared_ptr<spdlog::logger> APILogger = create_logger_();
+    class logger
+    {
+        public:
+            typedef std::shared_ptr<spdlog::logger> sptr;
+        private:
+    };
+
+    /**
+     * @brief Global logger used across all FDP scripts
+     * @author K. Zarebski (UKAEA)
+     *
+     */
+    extern const logger::sptr APILogger;
 } // namespace FDP
 
-#endif
+
+
+#endif

src/fdp.cxx

@@ -36,6 +36,9 @@ namespace FDP {
       ghc::filesystem::path script_file_path_() const {return config_->get_script_file_path();}
       std::string token_() const {return config_->get_token();}
       RESTAPI api_location_() {return config_->get_rest_api_location();}
+
+      impl& operator=(const impl& ) = delete;
+
   public:
       typedef std::shared_ptr< impl > sptr;
 
@@ -49,11 +52,11 @@ namespace FDP {
    ***************************************************************************/
   impl(const ghc::filesystem::path &config_file_path,
           const ghc::filesystem::path &file_system_path,
-          const std::string token,
+          const std::string& token,
           spdlog::level::level_enum log_level = spdlog::level::info,
           RESTAPI api_location = RESTAPI::LOCAL);
 
-  impl(const impl &dp);
+  //impl(const impl &dp) delete;
 
   /**
    * @brief Default Destructor
@@ -97,10 +100,10 @@ namespace FDP {
    * 
    * @return std::string 
    */
-  std::string get_code_run_uuid();
+  std::string get_code_run_uuid() const;
 
 };
-
+#if 0
 DataPipeline::impl::impl(const impl &dp)
 {
     impl( dp.config_file_path_()
@@ -109,11 +112,11 @@ DataPipeline::impl::impl(const impl &dp)
             , api_location_()
         );
 }
-
+#endif
 
 DataPipeline::impl::impl(const ghc::filesystem::path &config_file_path,
                     const ghc::filesystem::path &script_file_path,
-                    const std::string token,
+                    const std::string& token,
                     spdlog::level::level_enum log_level,
                     RESTAPI api_location)
 {
@@ -140,7 +143,7 @@ void FDP::DataPipeline::impl::finalise(){
     config_->finalise();
 }
 
-std::string FDP::DataPipeline::impl::get_code_run_uuid(){
+std::string FDP::DataPipeline::impl::get_code_run_uuid() const { 
     return config_->get_code_run_uuid();
 }
 

src/objects/api_object.cxx

@@ -90,10 +90,10 @@ namespace FDP {
         return obj_["url"].asString();
     }
 
-    std::string ApiObject::get_value_as_string(std::string key) const{
+    std::string ApiObject::get_value_as_string( const std::string& key) const{
         return obj_[key].asString();
     }
-    int ApiObject::get_value_as_int(std::string key) const{
+    int ApiObject::get_value_as_int(const std::string& key) const{
         return obj_[key].asInt();
     }
 

src/utilities/logging.cxx

@@ -1,11 +1,20 @@
 #include "fdp/utilities/logging.hxx"
 
 namespace FDP {
-std::shared_ptr<spdlog::logger> create_logger_() {
-  if (!spdlog::get("FDPAPI")) {
-    return spdlog::stdout_color_st("FDPAPI");
-  } else {
-    return spdlog::get("FDPAPI");
-  }
-}
-} // namespace FDP
+
+    /*! **************************************************************************
+     * @brief Create a the global logger pointer
+     * @author K. Zarebski (UKAEA)
+     *
+     * @return a shared pointer to the global logger instance
+     ****************************************************************************/
+    static logger::sptr create_logger() {
+        if (!spdlog::get("FDPAPI")) {
+            return spdlog::stdout_color_st("FDPAPI");
+        } else {
+            return spdlog::get("FDPAPI");
+        }
+    }
+
+    const logger::sptr APILogger = create_logger();
+} // namespace FDP