Cosmetic improvements for schema.hpp, schema_parser.hpp and subschema.hpp

include/valijson/schema.hpp

@@ -35,22 +35,28 @@ public:
       : Subschema(allocFn, freeFn),
         sharedEmptySubschema(newSubschema()) { }
 
+    // Disable copy construction
+    Schema(const Schema &) = delete;
+
+    // Disable copy assignment
+    Schema & operator=(const Schema &) = delete;
+
     /**
      * @brief  Clean up and free all memory managed by the Schema
      *
      * Note that any Subschema pointers created and returned by this Schema
      * should be considered invalid.
      */
-    virtual ~Schema()
+    ~Schema() override
     {
         sharedEmptySubschema->~Subschema();
-        freeFn(const_cast<Subschema *>(sharedEmptySubschema));
+        m_freeFn(const_cast<Subschema *>(sharedEmptySubschema));
-        sharedEmptySubschema = NULL;
+        sharedEmptySubschema = nullptr;
 
         try {
             for (auto subschema : subschemaSet) {
                 subschema->~Subschema();
-                freeFn(subschema);
+                m_freeFn(subschema);
             }
         } catch (const std::exception &e) {
             fprintf(stderr, "Caught an exception while destroying Schema: %s",
@@ -93,7 +99,7 @@ public:
             }
         } catch (...) {
             subschema->~Subschema();
-            freeFn(subschema);
+            m_freeFn(subschema);
             throw;
         }
 
@@ -160,15 +166,9 @@ public:
 
 private:
 
-    // Disable copy construction
-    Schema(const Schema &);
-
-    // Disable copy assignment
-    Schema & operator=(const Schema &);
-
     Subschema *newSubschema()
     {
-        void *ptr = allocFn(sizeof(Subschema));
+        void *ptr = m_allocFn(sizeof(Subschema));
         if (!ptr) {
             throw std::runtime_error(
                     "Failed to allocate memory for shared empty sub-schema");
@@ -177,7 +177,7 @@ private:
         try {
             return new (ptr) Subschema();
         } catch (...) {
-            freeFn(ptr);
+            m_freeFn(ptr);
             throw;
         }
     }
@@ -193,7 +193,7 @@ private:
                     "Cannot modify the shared empty sub-schema");
         }
 
-        Subschema *noConst = const_cast<Subschema*>(subschema);
+        auto *noConst = const_cast<Subschema*>(subschema);
         if (subschemaSet.find(noConst) == subschemaSet.end()) {
             throw std::runtime_error(
                     "Subschema pointer is not owned by this Schema instance");

include/valijson/schema_parser.hpp

@@ -49,7 +49,7 @@ public:
      *
      * @param  version  Version of JSON Schema that will be expected
      */
-    SchemaParser(const Version version = kDraft7)
+    explicit SchemaParser(const Version version = kDraft7)
       : version(version) { }
 
     /**
@@ -57,7 +57,7 @@ public:
      */
     ~SchemaParser()
     {
-        for (auto entry : constraintBuilders) {
+        for (const auto& entry : constraintBuilders) {
             delete entry.second;
         }
     }
@@ -143,7 +143,7 @@ public:
 
 private:
 
-    typedef std::vector<std::pair<std::string, const ConstraintBuilder *> >
+    typedef std::vector<std::pair<std::string, const ConstraintBuilder *>>
         ConstraintBuilders;
 
     ConstraintBuilders constraintBuilders;
@@ -2143,8 +2143,7 @@ private:
         if (patternProperties) {
             for (const Member m : patternProperties->getObject()) {
                 const std::string &pattern = m.first;
-                const std::string childPath = patternPropertiesPath + "/" +
+                const std::string childPath = patternPropertiesPath + "/" + pattern;
-                        pattern;
                 const Subschema *subschema = makeOrReuseSchema<AdapterType>(
                         rootSchema, rootNode, m.second, currentScope, childPath,
                         fetchDoc, parentSubschema, &pattern, docCache,
@@ -2305,9 +2304,7 @@ private:
         TypeConstraint constraint;
 
         if (node.maybeString()) {
-            const TypeConstraint::JsonType type =
+            const TypeConstraint::JsonType type = TypeConstraint::jsonTypeFromString(node.getString());
-                    TypeConstraint::jsonTypeFromString(node.getString());
-
             if (type == TypeConstraint::kAny && version == kDraft4) {
                 throw std::runtime_error(
                         "'any' type is not supported in version 4 schemas.");
@@ -2331,8 +2328,7 @@ private:
                     constraint.addNamedType(type);
 
                 } else if (v.maybeObject() && version == kDraft3) {
-                    const std::string childPath = nodePath + "/" +
+                    const std::string childPath = nodePath + "/" + std::to_string(index);
-                            std::to_string(index);
                     const Subschema *subschema = makeOrReuseSchema<AdapterType>(
                             rootSchema, rootNode, v, currentScope, childPath,
                             fetchDoc, nullptr, nullptr, docCache, schemaCache);
@@ -2367,8 +2363,7 @@ private:
      *          the caller, or nullptr if the boolean value is false.
      */
     template<typename AdapterType>
-    opt::optional<constraints::UniqueItemsConstraint>
+    opt::optional<constraints::UniqueItemsConstraint> makeUniqueItemsConstraint(const AdapterType &node)
-            makeUniqueItemsConstraint(const AdapterType &node)
     {
         if (node.isBool() || node.maybeBool()) {
             // If the boolean value is true, this function will return a pointer

include/valijson/subschema.hpp

@@ -22,6 +22,7 @@ namespace valijson {
 class Subschema
 {
 public:
+
     /// Typedef for custom new-/malloc-like function
     typedef void * (*CustomAlloc)(size_t size);
 
@@ -35,13 +36,19 @@ public:
     /// instances owned by a Schema.
     typedef std::function<bool (const Constraint &)> ApplyFunction;
 
+    // Disable copy construction
+    Subschema(const Subschema &) = delete;
+
+    // Disable copy assignment
+    Subschema & operator=(const Subschema &) = delete;
+
     /**
      * @brief  Construct a new Subschema object
      */
     Subschema()
-      : allocFn(::operator new)
+      : m_allocFn(::operator new)
-      , freeFn(::operator delete)
+      , m_freeFn(::operator delete)
-      , alwaysInvalid(false) { }
+      , m_alwaysInvalid(false) { }
 
     /**
      * @brief  Construct a new Subschema using custom memory management
@@ -53,9 +60,9 @@ public:
      *                  the `customAlloc` function
      */
     Subschema(CustomAlloc allocFn, CustomFree freeFn)
-      : allocFn(allocFn)
+      : m_allocFn(allocFn)
-      , freeFn(freeFn)
+      , m_freeFn(freeFn)
-      , alwaysInvalid(false) { }
+      , m_alwaysInvalid(false) { }
 
     /**
      * @brief  Clean up and free all memory managed by the Subschema
@@ -63,12 +70,12 @@ public:
     virtual ~Subschema()
     {
         try {
-            for (auto constConstraint : constraints) {
+            for (auto constConstraint : m_constraints) {
-                Constraint *constraint = const_cast<Constraint *>(constConstraint);
+                auto *constraint = const_cast<Constraint *>(constConstraint);
                 constraint->~Constraint();
-                freeFn(constraint);
+                m_freeFn(constraint);
             }
-            constraints.clear();
+            m_constraints.clear();
         } catch (const std::exception &e) {
             fprintf(stderr, "Caught an exception in Subschema destructor: %s",
                     e.what());
@@ -88,12 +95,12 @@ public:
      */
     void addConstraint(const Constraint &constraint)
     {
-        Constraint *newConstraint = constraint.clone(allocFn, freeFn);
+        Constraint *newConstraint = constraint.clone(m_allocFn, m_freeFn);
         try {
-            constraints.push_back(newConstraint);
+            m_constraints.push_back(newConstraint);
         } catch (...) {
             newConstraint->~Constraint();
-            freeFn(newConstraint);
+            m_freeFn(newConstraint);
             throw;
         }
     }
@@ -112,7 +119,7 @@ public:
     bool apply(ApplyFunction &applyFunction) const
     {
         bool allTrue = true;
-        for (const Constraint *constraint : constraints) {
+        for (const Constraint *constraint : m_constraints) {
             allTrue = allTrue && applyFunction(*constraint);
         }
 
@@ -131,7 +138,7 @@ public:
      */
     bool applyStrict(ApplyFunction &applyFunction) const
     {
-        for (const Constraint *constraint : constraints) {
+        for (const Constraint *constraint : m_constraints) {
             if (!applyFunction(*constraint)) {
                 return false;
             }
@@ -142,7 +149,7 @@ public:
 
     bool getAlwaysInvalid() const
     {
-        return alwaysInvalid;
+        return m_alwaysInvalid;
     }
 
     /**
@@ -154,8 +161,8 @@ public:
      */
     std::string getDescription() const
     {
-        if (description) {
+        if (m_description) {
-            return *description;
+            return *m_description;
         }
 
         throw std::runtime_error("Schema does not have a description");
@@ -170,8 +177,8 @@ public:
      */
     std::string getId() const
     {
-        if (id) {
+        if (m_id) {
-            return *id;
+            return *m_id;
         }
 
         throw std::runtime_error("Schema does not have an ID");
@@ -186,8 +193,8 @@ public:
      */
     std::string getTitle() const
     {
-        if (title) {
+        if (m_title) {
-            return *title;
+            return *m_title;
         }
 
         throw std::runtime_error("Schema does not have a title");
@@ -200,7 +207,7 @@ public:
      */
     bool hasDescription() const
     {
-        return static_cast<bool>(description);
+        return static_cast<bool>(m_description);
     }
 
     /**
@@ -210,7 +217,7 @@ public:
      */
     bool hasId() const
     {
-        return static_cast<bool>(id);
+        return static_cast<bool>(m_id);
     }
 
     /**
@@ -220,12 +227,12 @@ public:
      */
     bool hasTitle() const
     {
-        return static_cast<bool>(title);
+        return static_cast<bool>(m_title);
     }
 
     void setAlwaysInvalid(bool value)
     {
-        alwaysInvalid = value;
+        m_alwaysInvalid = value;
     }
 
     /**
@@ -240,12 +247,12 @@ public:
      */
     void setDescription(const std::string &description)
     {
-        this->description = description;
+        m_description = description;
     }
 
     void setId(const std::string &id)
     {
-        this->id = id;
+        m_id = id;
     }
 
     /**
@@ -260,36 +267,30 @@ public:
      */
     void setTitle(const std::string &title)
     {
-        this->title = title;
+        m_title = title;
     }
 
 protected:
 
-    CustomAlloc allocFn;
+    CustomAlloc m_allocFn;
 
-    CustomFree freeFn;
+    CustomFree m_freeFn;
 
 private:
 
-    // Disable copy construction
+    bool m_alwaysInvalid;
-    Subschema(const Subschema &);
-
-    // Disable copy assignment
-    Subschema & operator=(const Subschema &);
-
-    bool alwaysInvalid;
 
     /// List of pointers to constraints that apply to this schema.
-    std::vector<const Constraint *> constraints;
+    std::vector<const Constraint *> m_constraints;
 
     /// Schema description (optional)
-    opt::optional<std::string> description;
+    opt::optional<std::string> m_description;
 
     /// Id to apply when resolving the schema URI
-    opt::optional<std::string> id;
+    opt::optional<std::string> m_id;
 
     /// Title string associated with the schema (optional)
-    opt::optional<std::string> title;
+    opt::optional<std::string> m_title;
 };
 
 } // namespace valijson