3
\$\begingroup\$

Problem

  • We have a tool which produces output as json array
  • We have another tool which anticipates input as ndjson (newline delimited json)
  • I need to write a converter to transform json array to ndjson

Sample json array

[
  {
    "Property1": true,
    "Property2": 0.1,
    "Property3": "text"
  },
  {
    "Property1": false,
    "Property2": 0.2,
    "Property3": "not text"
  },
  {
    "Property1": true,
    "Property2": 3.14,
    "Property3": "sample"
  },
  {
    "Property1": false,
    "Property2": -51.0,
    "Property3": "Property3"
  }
]

Sample ndjson

{"Property1":true,"Property2":0.1,"Property3":"text"}
{"Property1":false,"Property2":0.2,"Property3":"not text"}
{"Property1":true,"Property2":3.14,"Property3":"sample"}
{"Property1":false,"Property2":-51.0,"Property3":"Property3"}

Design

  • Keep the converter as simple as possible
    • The converter should receive a source and a target file paths
    • The converter should not parse the data just reformat it
    • The converter should perform only some basic preliminary checks

Solution

  • I'm comfortable to implement this with Json.NET
    • But I wanted to practice with System.Text.Json
    • So, I've decided to implement it with the latter
  • I've added some comments to the code to help the reviewers
public static class JsonArrayToNDJsonConverter
{
    static JsonWriterOptions writerOptions = new() { Indented = false };
    public static void Convert(string sourcePath, string targetPath)
    {
        //Preliminary checks on source
        if (!File.Exists(sourcePath))
            throw new FileNotFoundException("Source file is not found");
        using var sourceFile = new FileStream(sourcePath, FileMode.Open);

        var jsonDocument = JsonDocument.Parse(sourceFile);
        if (jsonDocument.RootElement.ValueKind != JsonValueKind.Array)
            throw new InvalidOperationException("Json Array must be the source");

        //Not that graceful handling of file existence
        if (File.Exists(targetPath))
            File.Delete(targetPath);
        using var targetFile = new FileStream(targetPath, FileMode.CreateNew);

        using var targetFileWriter = new StreamWriter(targetFile);
        using var jsonObjectWithoutIndentation = new MemoryStream();
        foreach (var jsonObject in jsonDocument.RootElement.EnumerateArray())
        {
            //Write json object without indentation into a memorystream
            var jsonObjectStreamWriter = new Utf8JsonWriter(jsonObjectWithoutIndentation, writerOptions);
            jsonObject.WriteTo(jsonObjectStreamWriter);
            jsonObjectStreamWriter.Flush();

            //Write memorystream to target
            var singleLinedJson = Encoding.UTF8.GetString(jsonObjectWithoutIndentation.ToArray());
            targetFileWriter.WriteLine(singleLinedJson);

            //Reuse memory stream
            jsonObjectWithoutIndentation.Position = 0;
            jsonObjectWithoutIndentation.SetLength(0);
        }
    }
}
\$\endgroup\$

3 Answers 3

2
\$\begingroup\$
  • always increase readability by using access modifiers.
  • Convert is misleading, as you're reading and writing files, so describing the function is very useful to understand what's going on inside the method.
  • File.Delete(targetPath) is not necessary, as you can use FileMode.Create which will create the file if not exists, and override it if exists.
  • use JsonException instead of InvalidOperationException.
  • Utf8JsonWriter should be outside the loop and it's Disposable.
  • You can always use File.ReadAllText or File.WriteAllText instead for small to medium size files, as shortcut, however, for large files, I advice you using the proper streaming techniques (such as reading file in chunks) instead of reading everything.
  • use the proper naming for objects such as memoryStream instead of jsonObjectWithoutIndentation and jsonWriter instead of jsonObjectStreamWriter to avoid confusion.
  • use Flush() and Reset() all together with the Utf8JsonWriter to reset the stream. using Flush() alone would case some exceptions, as there are some stream counters are only reset by the Reset() and not by Flush() like BytesCommitted.
  • you can use FileStream with Utf8JsonWriter directly, you just need to add a new line after each write.

here is a revised version that would shows the above points :

public static class JsonArrayToNDJsonConverter
{
    private static JsonWriterOptions _jsonWriterOptionNoIndentation = new() { Indented = false };
   
    private static byte[] _newLineBytes = Encoding.UTF8.GetBytes(Environment.NewLine);

    public static void ConvertJsonArrayFileToNDArray(string sourcePath, string targetPath)
    {
        if (string.IsNullOrWhiteSpace(sourcePath))
            throw new ArgumentNullException(nameof(sourcePath));
        
        if (string.IsNullOrWhiteSpace(targetPath))
            throw new ArgumentNullException(nameof(targetPath));

        if(!File.Exists(sourcePath))
            throw new FileNotFoundException("Source file is not found");
    
        var sourceFile = File.ReadAllText(sourcePath);

        var jsonDocument = JsonDocument.Parse(sourceFile);
        
        if (jsonDocument.RootElement.ValueKind != JsonValueKind.Array)
            throw new JsonException("Json Array must be the source");

        using var targetFileStream = new FileStream(targetPath, FileMode.Create);
        using var jsonWriter = new Utf8JsonWriter(targetFileStream, _jsonWriterOptionNoIndentation);
        foreach (var jsonObject in jsonDocument.RootElement.EnumerateArray())
        {
            jsonObject.WriteTo(jsonWriter);
            jsonWriter.Flush();
            jsonWriter.Reset();
            targetFileStream.Write(_newLineBytes);
        }
    }
}

Though I think it would be better if we just divide all that into several methods and inside one helper class, to give a better reusability. Also, it would be more effective if we just target JsonArray instead of JsonDocument as you're only targting the JsonArray so using JsonNode and JsonArray would be enough. Example :

public static class JsonHelper
{
    public static JsonNode LoadFromFile(string filePath)
    {
        if (string.IsNullOrWhiteSpace(filePath))
            throw new ArgumentNullException(nameof(filePath));

        if (!File.Exists(filePath))
            throw new FileNotFoundException("Source file is not found");

        return JsonNode.Parse(File.ReadAllText(filePath));
    }

    public static void WriteToFileAsNDArray(this JsonArray jsonArray, string targetPath)
    {
        if (jsonArray == null)
            throw new ArgumentNullException(nameof(jsonArray));

        if (string.IsNullOrWhiteSpace(targetPath))
            throw new ArgumentNullException(nameof(targetPath));

        // remember this will throw an exception if the target path is invalid
        // if the file already exists, it'll be overridden
        File.WriteAllText(targetPath, jsonArray.ToJsonStringNDArray());
    }
}

public static class JsonNodeExtensions
{
    private static JsonWriterOptions _jsonWriterOptionNoIndentation = new() { Indented = false };
    
    private static byte[] _newLineBytes = Encoding.UTF8.GetBytes(Environment.NewLine);

    public static string ToJsonStringNDArray(this JsonArray jsonArray)
    {
        if (jsonArray == null)
            throw new ArgumentNullException(nameof(jsonArray));

        using var memoryStream = new MemoryStream();
        using var jsonWriter = new Utf8JsonWriter(memoryStream, _jsonWriterOptionNoIndentation);
        foreach (var jsonObject in jsonArray)
        {
            jsonObject.WriteTo(jsonWriter);
            jsonWriter.Flush();
            jsonWriter.Reset();
            memoryStream.Write(_newLineBytes);
        }

        return Encoding.UTF8.GetString(memoryStream.ToArray());
    }
}

Usage :

JsonHelper.LoadFromFile(sourcePath)
          .AsArray()
          .WriteToFileAsNDArray(targetPath);
\$\endgroup\$
4
  • \$\begingroup\$ Thanks for the review. I've used the Utf8JsonWriter inside the loop, because I was not aware of the Reset call and I couldn't reuse the writer for multiple JsonElements. \$\endgroup\$ May 5 at 10:55
  • \$\begingroup\$ use the proper naming for objects such as memoryStream instead of jsonObjectWithoutIndentation > I can argue with that. Naming a variable to memoryStream, stringBuilder, jArray, etc... does not tell anything about the data that is stored inside them. But I think this is more like a personal preference than following some well-known guidance. \$\endgroup\$ May 5 at 11:01
  • 1
    \$\begingroup\$ @PeterCsala JsonObject is a class under System.Text.Json what I meant is just to use a naming that is related to the object that is currently used. so jsonObjectWithoutIndentation at first glance, would give an impression that it's a JsonObject and not a MemoryStream. \$\endgroup\$
    – iSR5
    May 5 at 12:17
  • 1
    \$\begingroup\$ @PeterCsala I doubt that both System.Text.Json and Json.NET with their Converter would do you any good for NDArray. As this would need to alter the serializer behavior, and I don't think it would be a wise idea to go that far. So, your approach is the easiest and safest way to handle it. \$\endgroup\$
    – iSR5
    May 5 at 12:21
2
\$\begingroup\$
  • If sourcePath is null the method would throw an FileNotFoundException as well, because File.Exists() will return false in such a case. You would have serious problems to find the cause of a FileNotFoundException if you would see in the filesystem that the file exists but your code is telling otherwise because you assume having passed the correct filename.

  • If you are sure that targetPath is a valid filename, you won't need to call File.Exists() because File.Delete() won't throw an exception if the file doesn't exists.

  • Calling MemoryStream.SetLength(0) will set its Position to 0 as well.

  • Using braces {} althought they migth be optional will prevent hidden bugs.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the suggestions. Since I haven't received any comment on the usage of the System.Text.Json I assume that part is okayish. \$\endgroup\$ May 4 at 13:39
  • \$\begingroup\$ I can't tell you that because I never used System.Text.Json. \$\endgroup\$
    – Heslacher
    May 4 at 13:43
1
\$\begingroup\$

Parameters

It's more natural and flexible to just receive the input and output streams as parameters. The conversion doesn't really care if it's reading the JSON from a file or some other source (e.g. HTTP request).

Memory Usage

Loading the entire JSON into memory is not ideal if the content is sufficiently large. This transformation can be easily done with \$\mathcal{O}(1)\$ memory by using streaming techniques.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.