Code review comments:

- const methods in yaml parser
- std::make_unique
- exceptions

Author: marcalff

sdk/include/opentelemetry/sdk/configuration/document_node.h

@@ -32,25 +32,26 @@ class DocumentNode
 
   virtual std::string Key() const = 0;
 
-  virtual bool AsBoolean()        = 0;
-  virtual std::size_t AsInteger() = 0;
-  virtual double AsDouble()       = 0;
-  virtual std::string AsString()  = 0;
+  virtual bool AsBoolean() const        = 0;
+  virtual std::size_t AsInteger() const = 0;
+  virtual double AsDouble() const       = 0;
+  virtual std::string AsString() const  = 0;
 
-  virtual std::unique_ptr<DocumentNode> GetRequiredChildNode(const std::string &name) = 0;
-  virtual std::unique_ptr<DocumentNode> GetChildNode(const std::string &name)         = 0;
+  virtual std::unique_ptr<DocumentNode> GetRequiredChildNode(const std::string &name) const = 0;
+  virtual std::unique_ptr<DocumentNode> GetChildNode(const std::string &name) const         = 0;
 
-  virtual bool GetRequiredBoolean(const std::string &name)             = 0;
-  virtual bool GetBoolean(const std::string &name, bool default_value) = 0;
+  virtual bool GetRequiredBoolean(const std::string &name) const             = 0;
+  virtual bool GetBoolean(const std::string &name, bool default_value) const = 0;
 
-  virtual std::size_t GetRequiredInteger(const std::string &name)                    = 0;
-  virtual std::size_t GetInteger(const std::string &name, std::size_t default_value) = 0;
+  virtual std::size_t GetRequiredInteger(const std::string &name) const                    = 0;
+  virtual std::size_t GetInteger(const std::string &name, std::size_t default_value) const = 0;
 
-  virtual double GetRequiredDouble(const std::string &name)               = 0;
-  virtual double GetDouble(const std::string &name, double default_value) = 0;
+  virtual double GetRequiredDouble(const std::string &name) const               = 0;
+  virtual double GetDouble(const std::string &name, double default_value) const = 0;
 
-  virtual std::string GetRequiredString(const std::string &name)                           = 0;
-  virtual std::string GetString(const std::string &name, const std::string &default_value) = 0;
+  virtual std::string GetRequiredString(const std::string &name) const  = 0;
+  virtual std::string GetString(const std::string &name,
+                                const std::string &default_value) const = 0;
 
   virtual DocumentNodeConstIterator begin() const = 0;
   virtual DocumentNodeConstIterator end() const   = 0;
@@ -62,12 +63,12 @@ class DocumentNode
   virtual PropertiesNodeConstIterator end_properties() const   = 0;
 
 protected:
-  std::string DoSubstitution(const std::string &text);
-  std::string DoOneSubstitution(const std::string &text);
+  std::string DoSubstitution(const std::string &text) const;
+  std::string DoOneSubstitution(const std::string &text) const;
 
-  bool BooleanFromString(const std::string &value);
-  std::size_t IntegerFromString(const std::string &value);
-  double DoubleFromString(const std::string &value);
+  bool BooleanFromString(const std::string &value) const;
+  std::size_t IntegerFromString(const std::string &value) const;
+  double DoubleFromString(const std::string &value) const;
 };
 
 class DocumentNodeConstIteratorImpl

sdk/include/opentelemetry/sdk/configuration/ryml_document_node.h

@@ -29,25 +29,25 @@ class RymlDocumentNode : public DocumentNode
 
   std::string Key() const override;
 
-  bool AsBoolean() override;
-  std::size_t AsInteger() override;
-  double AsDouble() override;
-  std::string AsString() override;
+  bool AsBoolean() const override;
+  std::size_t AsInteger() const override;
+  double AsDouble() const override;
+  std::string AsString() const override;
 
-  std::unique_ptr<DocumentNode> GetRequiredChildNode(const std::string &name) override;
-  std::unique_ptr<DocumentNode> GetChildNode(const std::string &name) override;
+  std::unique_ptr<DocumentNode> GetRequiredChildNode(const std::string &name) const override;
+  std::unique_ptr<DocumentNode> GetChildNode(const std::string &name) const override;
 
-  bool GetRequiredBoolean(const std::string &name) override;
-  bool GetBoolean(const std::string &name, bool default_value) override;
+  bool GetRequiredBoolean(const std::string &name) const override;
+  bool GetBoolean(const std::string &name, bool default_value) const override;
 
-  std::size_t GetRequiredInteger(const std::string &name) override;
-  std::size_t GetInteger(const std::string &name, std::size_t default_value) override;
+  std::size_t GetRequiredInteger(const std::string &name) const override;
+  std::size_t GetInteger(const std::string &name, std::size_t default_value) const override;
 
-  double GetRequiredDouble(const std::string &name) override;
-  double GetDouble(const std::string &name, double default_value) override;
+  double GetRequiredDouble(const std::string &name) const override;
+  double GetDouble(const std::string &name, double default_value) const override;
 
-  std::string GetRequiredString(const std::string &name) override;
-  std::string GetString(const std::string &name, const std::string &default_value) override;
+  std::string GetRequiredString(const std::string &name) const override;
+  std::string GetString(const std::string &name, const std::string &default_value) const override;
 
   DocumentNodeConstIterator begin() const override;
   DocumentNodeConstIterator end() const override;
@@ -59,8 +59,8 @@ class RymlDocumentNode : public DocumentNode
   PropertiesNodeConstIterator end_properties() const override;
 
 private:
-  ryml::ConstNodeRef GetRequiredRymlChildNode(const std::string &name);
-  ryml::ConstNodeRef GetRymlChildNode(const std::string &name);
+  ryml::ConstNodeRef GetRequiredRymlChildNode(const std::string &name) const;
+  ryml::ConstNodeRef GetRymlChildNode(const std::string &name) const;
 
   ryml::ConstNodeRef node_;
   std::size_t depth_;