Improve Json::Value:

o Increase test coverage.
o Remove unused and/or broken interfaces.

.travis.yml

@@ -228,6 +228,7 @@ matrix:
             - bash
             - ninja
             - cmake
+            - wget
             - [email protected]
           update: true
       install:

src/ripple/json/impl/json_value.cpp

@@ -122,21 +122,6 @@ Value::CZString::~CZString ()
         valueAllocator ()->releaseMemberName ( const_cast<char*> ( cstr_ ) );
 }
 
-void
-Value::CZString::swap ( CZString& other ) noexcept
-{
-    std::swap ( cstr_, other.cstr_ );
-    std::swap ( index_, other.index_ );
-}
-
-Value::CZString&
-Value::CZString::operator = ( const CZString& other )
-{
-    CZString temp ( other );
-    swap ( temp );
-    return *this;
-}
-
 bool
 Value::CZString::operator< ( const CZString& other ) const
 {
@@ -250,24 +235,12 @@ Value::Value ( const char* value )
     value_.string_ = valueAllocator ()->duplicateStringValue ( value );
 }
 
-
-Value::Value ( const char* beginValue,
-               const char* endValue )
-    : type_ ( stringValue )
-    , allocated_ ( true )
-{
-    value_.string_ = valueAllocator ()->duplicateStringValue ( beginValue,
-                     UInt (endValue - beginValue) );
-}
-
-
 Value::Value ( std::string const& value )
     : type_ ( stringValue )
     , allocated_ ( true )
 {
     value_.string_ = valueAllocator ()->duplicateStringValue ( value.c_str (),
                      (unsigned int)value.length () );
-
 }
 
 Value::Value ( const StaticString& value )
@@ -562,7 +535,10 @@ Value::asInt () const
         return value_.bool_ ? 1 : 0;
 
     case stringValue:
-        return beast::lexicalCastThrow <int> (value_.string_);
+        {
+            char const* const str {value_.string_ ? value_.string_ : ""};
+            return beast::lexicalCastThrow <int> (str);
+        }
 
     case arrayValue:
     case objectValue:
@@ -598,7 +574,10 @@ Value::asUInt () const
         return value_.bool_ ? 1 : 0;
 
     case stringValue:
-        return beast::lexicalCastThrow <unsigned int> (value_.string_);
+        {
+            char const* const str {value_.string_ ? value_.string_ : ""};
+            return beast::lexicalCastThrow <unsigned int> (str);
+        }
 
     case arrayValue:
     case objectValue:
@@ -802,30 +781,6 @@ Value::clear ()
     }
 }
 
-void
-Value::resize ( UInt newSize )
-{
-    JSON_ASSERT ( type_ == nullValue  ||  type_ == arrayValue );
-
-    if ( type_ == nullValue )
-        *this = Value ( arrayValue );
-
-    UInt oldSize = size ();
-
-    if ( newSize == 0 )
-        clear ();
-    else if ( newSize > oldSize )
-        (*this)[ newSize - 1 ];
-    else
-    {
-        for ( UInt index = newSize; index < oldSize; ++index )
-            value_.map_->erase ( index );
-
-        assert ( size () == newSize );
-    }
-}
-
-
 Value&
 Value::operator[] ( UInt index )
 {
@@ -954,6 +909,12 @@ Value::append ( const Value& value )
     return (*this)[size ()] = value;
 }
 
+Value&
+Value::append ( Value&& value )
+{
+    return (*this)[size ()] = std::move(value);
+}
+
 
 Value
 Value::get ( const char* key,

src/ripple/json/json_value.h

@@ -46,17 +46,9 @@ enum ValueType
     objectValue    ///< object value (collection of name/value pairs).
 };
 
-enum CommentPlacement
-{
-    commentBefore = 0,        ///< a comment placed on the line before a value
-    commentAfterOnSameLine,   ///< a comment just after a value on the same line
-    commentAfter,             ///< a comment on the line after a value (only make sense for root value)
-    numberOfCommentPlacement
-};
-
 /** \brief Lightweight wrapper to tag static string.
  *
- * Value constructor and objectValue member assignement takes advantage of the
+ * Value constructor and objectValue member assignment takes advantage of the
  * StaticString and avoid the cost of string duplication when storing the
  * string or the member name.
  *
@@ -92,8 +84,6 @@ class StaticString
 
 inline bool operator== (StaticString x, StaticString y)
 {
-    // TODO(tom): could I use x != y here because StaticStrings are supposed to
-    // be unique?
     return strcmp (x.c_str(), y.c_str()) == 0;
 }
 
@@ -180,14 +170,13 @@ class Value
         CZString ( const char* cstr, DuplicationPolicy allocate );
         CZString ( const CZString& other );
         ~CZString ();
-        CZString& operator = ( const CZString& other );
+        CZString& operator = ( const CZString& other ) = delete;
         bool operator< ( const CZString& other ) const;
         bool operator== ( const CZString& other ) const;
         int index () const;
         const char* c_str () const;
         bool isStaticString () const;
     private:
-        void swap ( CZString& other ) noexcept;
         const char* cstr_;
         int index_;
     };
@@ -216,7 +205,6 @@ class Value
     Value ( UInt value );
     Value ( double value );
     Value ( const char* value );
-    Value ( const char* beginValue, const char* endValue );
     /** \brief Constructs a value from a static string.
 
      * Like other value string constructor but do not duplicate the string for
@@ -239,8 +227,6 @@ class Value
     Value ( Value&& other ) noexcept;
 
     /// Swap values.
-    /// \note Currently, comments are intentionally not swapped, for
-    /// both logic and efficiency.
     void swap ( Value& other ) noexcept;
 
     ValueType type () const;
@@ -284,13 +270,6 @@ class Value
     /// \post type() is unchanged
     void clear ();
 
-    /// Resize the array to size elements.
-    /// New elements are initialized to null.
-    /// May only be called on nullValue or arrayValue.
-    /// \pre type() is arrayValue or nullValue
-    /// \post type() is arrayValue
-    void resize ( UInt size );
-
     /// Access an array element (zero based index ).
     /// If the array contains less than index element, then null value are inserted
     /// in the array so that its size is index+1.
@@ -311,6 +290,7 @@ class Value
     ///
     /// Equivalent to jsonvalue[jsonvalue.size()] = value;
     Value& append ( const Value& value );
+    Value& append ( Value&& value );
 
     /// Access an object value by name, create a null member if it does not exist.
     Value& operator[] ( const char* key );
@@ -362,10 +342,6 @@ class Value
     /// \post if type() was nullValue, it remains nullValue
     Members getMemberNames () const;
 
-    bool hasComment ( CommentPlacement placement ) const;
-    /// Include delimiters and embedded newlines.
-    std::string getComment ( CommentPlacement placement ) const;
-
     std::string toStyledString () const;
 
     const_iterator begin () const;
@@ -468,11 +444,6 @@ class ValueIteratorBase
         return !isEqual ( other );
     }
 
-    difference_type operator - ( const SelfType& other ) const
-    {
-        return computeDistance ( other );
-    }
-
     /// Return either the index or the member name of the referenced value as a Value.
     Value key () const;
 

src/ripple/json/json_writer.h

@@ -75,9 +75,7 @@ class FastWriter : public WriterBase
  *     - otherwise, it the values do not fit on one line, or the array contains
  *       object or non empty array, then print one value per line.
  *
- * If the Value have comments then they are outputed according to their #CommentPlacement.
- *
- * \sa Reader, Value, Value::setComment()
+ * \sa Reader, Value
  */
 class StyledWriter: public WriterBase
 {
@@ -127,10 +125,8 @@ class StyledWriter: public WriterBase
  *     - otherwise, it the values do not fit on one line, or the array contains
  *       object or non empty array, then print one value per line.
  *
- * If the Value have comments then they are outputed according to their #CommentPlacement.
- *
  * \param indentation Each level will be indented by this amount extra.
- * \sa Reader, Value, Value::setComment()
+ * \sa Reader, Value
  */
 class StyledStreamWriter
 {