Mercurial > octave
diff libinterp/corefcn/jsondecode.cc @ 28750:80857685105b
Code review for jsonencode/jsondecode functions.
* jsondecode.cc, jsonencode.cc: Don't use period at end of error() text.
Don't explicitly call octave_value() on objects returned from functions declared
to return octave_value. Use true/false for bool values rather than 1/0.
Update comments and in-line documentation. Don't declare and initialize
multiple variabes on the same line. Use C++ R"()" syntax to avoid escaping
double quotes so frequently. Use Octave conventions for spacing between
literals and '('. Rewrite docstring and ues @multitable for clearer
presentation of conversions. Add BISTS tests for input validation.
* jsondecode.cc (Fjsondecode): Change reported offset error to use 1-based
indexing consistent with Octave arrays.
* jsonencode.cc (encode_numeric): Replace multiple tests for isnan, isinf, isna
with single call to isfinite.
* strings.txi: List jsonencode function before jsondecode. Change @menu
structure to reflect new ordering.
author | Rik <rik@octave.org> |
---|---|
date | Thu, 17 Sep 2020 05:07:02 -0700 |
parents | 34696240591e |
children | b5461b1dc0ca |
line wrap: on
line diff
--- a/libinterp/corefcn/jsondecode.cc Wed Sep 16 13:15:35 2020 -0700 +++ b/libinterp/corefcn/jsondecode.cc Thu Sep 17 05:07:02 2020 -0700 @@ -71,7 +71,7 @@ else if (val.IsDouble ()) return octave_value (val.GetDouble ()); else - error ("jsondecode.cc: Unidentified type."); + error ("jsondecode: unidentified type"); } //! Decodes a JSON object into a scalar struct. @@ -93,15 +93,19 @@ decode_object (const rapidjson::Value& val, const octave_value_list& options) { octave_scalar_map retval; + + // Validator function to guarantee legitimate variable name. + std::string fcn_name = "matlab.lang.makeValidName"; + for (const auto& pair : val.GetObject ()) { - std::string fcn_name = "matlab.lang.makeValidName"; octave_value_list args = octave_value_list (pair.name.GetString ()); args.append (options); std::string validName = octave::feval (fcn_name,args)(0).string_value (); retval.assign (validName, decode (pair.value, options)); } - return octave_value (retval); + + return retval; } //! Decodes a JSON array that contains only numerical or null values @@ -189,7 +193,7 @@ return retval; } -//! Decodes a JSON array that contains only objects into a Cell or a struct array +//! Decodes a JSON array that contains only objects into a Cell or struct array //! depending on the similarity of the objects' keys. //! //! @param val JSON value that is guaranteed to be an object array. @@ -220,14 +224,16 @@ { Cell struct_cell = decode_string_and_mixed_array (val, options).cell_value (); string_vector field_names = struct_cell(0).scalar_map_value ().fieldnames (); + bool same_field_names = true; for (octave_idx_type i = 1; i < struct_cell.numel (); ++i) if (field_names.std_list () != struct_cell(i).scalar_map_value ().fieldnames ().std_list ()) { - same_field_names = 0; + same_field_names = false; break; } + if (same_field_names) { octave_map struct_array; @@ -238,14 +244,14 @@ value(k) = struct_cell(k).scalar_map_value ().getfield (field_names(i)); struct_array.assign (field_names(i), value); } - return octave_value (struct_array); + return struct_array; } else return struct_cell; } //! Decodes a JSON array that contains only arrays into a Cell or an NDArray -//! depending on the dimensions and the elements' type of the sub arrays. +//! depending on the dimensions and element types of the sub-arrays. //! //! @param val JSON value that is guaranteed to be an array of arrays. //! @param options @c ReplacementStyle and @c Prefix options with their values. @@ -274,26 +280,28 @@ const octave_value_list& options) { // Some arrays should be decoded as NDArrays and others as cell arrays - Cell cell = decode_string_and_mixed_array(val, options).cell_value (); - // Only arrays with sub arrays of booleans and numericals will return NDArray + Cell cell = decode_string_and_mixed_array (val, options).cell_value (); + + // Only arrays with sub-arrays of booleans and numericals will return NDArray bool is_bool = cell(0).is_bool_matrix (); dim_vector sub_array_dims = cell(0).dims (); octave_idx_type sub_array_ndims = cell(0).ndims (); octave_idx_type cell_numel = cell.numel (); for (octave_idx_type i = 0; i < cell_numel; ++i) { - // If one element is cell return the cell array as at least one of - // the sub arrays area either an array of: strings, objects or mixed array + // If one element is cell return the cell array as at least one of the + // sub-arrays area either an array of: strings, objects or mixed array if (cell(i).iscell ()) return cell; - // If not the same dim of elements or dim = 0 return cell array + // If not the same dim of elements or dim = 0, return cell array if (cell(i).dims () != sub_array_dims || sub_array_dims == dim_vector ()) return cell; - // If not numeric sub arrays only or bool - // sub arrays only return cell array - if(cell(i).is_bool_matrix () != is_bool) + // If not numeric sub-arrays only or bool sub-arrays only, + // return cell array + if (cell(i).is_bool_matrix () != is_bool) return cell; } + // Calculate the dims of the output array dim_vector array_dims; array_dims.resize (sub_array_ndims + 1); @@ -310,7 +318,7 @@ return array; } -//! Decodes any type of JSON arrays. This function only serves as an interface +//! Decodes any type of JSON arrays. This function only serves as an interface //! by choosing which function to call from the previous functions. //! //! @param val JSON value that is guaranteed to be an array. @@ -331,28 +339,31 @@ { // Handle empty arrays if (val.Empty ()) - return NDArray (dim_vector (0,0)); + return NDArray (); // Compare with other elements to know if the array has multiple types rapidjson::Type array_type = val[0].GetType (); - // Check if the array is numeric and if it has multible types - bool same_type = 1, is_numeric = 1; + // Check if the array is numeric and if it has multiple types + bool same_type = true; + bool is_numeric = true; for (const auto& elem : val.GetArray ()) { rapidjson::Type current_elem_type = elem.GetType (); if (is_numeric && ! (current_elem_type == rapidjson::kNullType || current_elem_type == rapidjson::kNumberType)) - is_numeric = 0; + is_numeric = false; if (same_type && (current_elem_type != array_type)) // RapidJSON doesn't have kBoolean Type it has kTrueType and kFalseType if (! ((current_elem_type == rapidjson::kTrueType && array_type == rapidjson::kFalseType) || (current_elem_type == rapidjson::kFalseType && array_type == rapidjson::kTrueType))) - same_type = 0; + same_type = false; } + if (is_numeric) return decode_numeric_array (val); + if (same_type && (array_type != rapidjson::kStringType)) { if (array_type == rapidjson::kTrueType @@ -363,13 +374,13 @@ else if (array_type == rapidjson::kArrayType) return decode_array_of_arrays (val, options); else - error ("jsondecode.cc: Unidentified type."); + error ("jsondecode: unidentified type"); } else return decode_string_and_mixed_array (val, options); } -//! Decodes any JSON value. This function only serves as an interface +//! Decodes any JSON value. This function only serves as an interface //! by choosing which function to call from the previous functions. //! //! @param val JSON value. @@ -397,77 +408,52 @@ else if (val.IsObject ()) return decode_object (val, options); else if (val.IsNull ()) - return NDArray (dim_vector (0,0)); + return NDArray (); else if (val.IsArray ()) return decode_array (val, options); else - error ("jsondecode.cc: Unidentified type."); + error ("jsondecode: unidentified type"); } #endif DEFUN (jsondecode, args, , doc: /* -*- texinfo -*- -@deftypefn {} {@var{object} =} jsondecode (@var{json}) -@deftypefnx {} {@var{object} =} jsondecode (@var{json}, "ReplacementStyle", @var{rs}) -@deftypefnx {} {@var{object} =} jsondecode (@var{json}, "Prefix", @var{pfx}) -@deftypefnx {} {@var{object} =} jsondecode (@var{json}, @dots{}) +@deftypefn {} {@var{object} =} jsondecode (@var{JSON_txt}) +@deftypefnx {} {@var{object} =} jsondecode (@dots{}, "ReplacementStyle", @var{rs}) +@deftypefnx {} {@var{object} =} jsondecode (@dots{}, "Prefix", @var{pfx}) Decode text that is formatted in JSON. -The input @var{json} is a string that contains JSON text. -The output @var{object} is an Octave object that contains the result -of decoding @var{json}. +The input @var{JSON_txt} is a string that contains JSON text. + +The output @var{object} is an Octave object that contains the result of +decoding @var{JSON_txt}. For more information about the options @qcode{"ReplacementStyle"} and @qcode{"Prefix"}, see @ref{XREFmatlab_lang_makeValidName,,matlab.lang.makeValidName}. --NOTE: It is not guaranteed to get the same JSON text if you decode -and then encode it as some names may change by -@code{matlab.lang.makeValidName}. +NOTE: Decoding and encoding JSON text is not guaranteed to reproduce the +original text as some names may be changed by @code{matlab.lang.makeValidName}. This table shows the conversions from JSON data types to Octave data types: -@table @asis -@item @qcode{"Boolean"} -Scalar @qcode{"logical"} - -@item @qcode{"Number"} -Scalar @qcode{"double"} - -@item @qcode{"String"} -@qcode{"Vector"} of chars - -@item JSON @qcode{"Object"} -Scalar @qcode{"struct"} (field names of the struct may be different from -the keys of the JSON object due to -@ref{XREFmatlab_lang_makeValidName,,matlab.lang.makeValidName}) - -@item @qcode{"Array"} of different data types -@qcode{"Cell"} array - -@item @qcode{"Array"} of booleans -@qcode{"Array"} of logicals - -@item @qcode{"Array"} of numbers -@qcode{"Array"} of doubles - -@item @qcode{"Array"} of strings -@qcode{"Cell"} array of vectors of chars - -@item @qcode{"Array"} of JSON objects (All objects have the same field names) -@qcode{"Struct array"} - -@item @qcode{"Array"} of JSON objects (Objects have different field names) -@qcode{"Cell"} array of scalar structs - -@item @qcode{"null"} inside a numeric array -@qcode{"NaN"} - -@item @qcode{"null"} inside a non-numeric array -Empty @qcode{"Array"} of doubles (@qcode{"[]"}) -@end table +@multitable @columnfractions 0.50 0.50 +@headitem JSON data type @tab Octave data type +@item Boolean @tab scalar logical +@item Number @tab scalar double +@item String @tab vector of characters +@item Object @tab scalar struct (field names of the struct may be different from the keys of the JSON object due to @code{matlab_lang_makeValidName} +@item null, inside a numeric array @tab @code{NaN} +@item null, inside a non-numeric array @tab empty double array @code{[]} +@item Array, of different data types @tab cell array +@item Array, of Booleans @tab logical array +@item Array, of Numbers @tab double array +@item Array, of Strings @tab cell array of character vectors (@code{cellstr}) +@item Array of Objects, same field names @tab struct array +@item Array of Objects, different field names @tab cell array of scalar structs +@end multitable Examples: @@ -498,7 +484,8 @@ @end group @group -jsondecode ('@{"nu#m#ber": 7, "s#tr#ing": "hi"@}', 'ReplacementStyle', 'delete') +jsondecode ('@{"nu#m#ber": 7, "s#tr#ing": "hi"@}', ... + 'ReplacementStyle', 'delete') @result{} scalar structure containing the fields: number = 7 @@ -520,26 +507,27 @@ #if defined (HAVE_RAPIDJSON) int nargin = args.length (); + // makeValidName options are pairs, the number of arguments must be odd. if (! (nargin % 2)) print_usage (); - if(! args(0).is_string ()) - error ("jsondecode: The input must be a character string"); + if (! args(0).is_string ()) + error ("jsondecode: JSON_TXT must be a character string"); - std::string json = args (0).string_value (); + std::string json = args(0).string_value (); rapidjson::Document d; // DOM is chosen instead of SAX as SAX publishes events to a handler that // decides what to do depending on the event only. This will cause a // problem in decoding JSON arrays as the output may be an array or a cell // and that doesn't only depend on the event (startArray) but also on the // types of the elements inside the array. - d.Parse <rapidjson::kParseNanAndInfFlag>(json.c_str ()); + d.Parse <rapidjson::kParseNanAndInfFlag> (json.c_str ()); if (d.HasParseError ()) - error("jsondecode: Parse error at offset %u: %s\n", - static_cast<unsigned int> (d.GetErrorOffset ()), - rapidjson::GetParseError_En (d.GetParseError ())); + error ("jsondecode: parse error at offset %u: %s\n", + static_cast<unsigned int> (d.GetErrorOffset ()) + 1, + rapidjson::GetParseError_En (d.GetParseError ())); return decode (d, args.slice (1, nargin - 1)); @@ -551,3 +539,16 @@ #endif } + +/* +FIXME: Need BIST tests for decoding each data type. +##%!testif HAVE_RAPIDJSON + +## Input validation tests +%!testif HAVE_RAPIDJSON +%! fail ("jsondecode ()"); +%! fail ("jsondecode ('1', 2)"); +%! fail ("jsondecode (1)", "JSON_TXT must be a character string"); +%! fail ("jsondecode ('12-')", "parse error at offset 3"); + +*/