Bunny83 / SimpleJSON

A simple JSON parser in C#
MIT License
735 stars 294 forks source link

Example of using foreach on JSONNode ? #24

Closed jasonhbartlett closed 4 years ago

jasonhbartlett commented 5 years ago

Hello there, I found your very useful SimpleJSON script and I expect it will save me a lot of time. Thank you! I wondered if you could demonstrate how to use it in a foreach and switch statement way. Something like this:

       var UserSettings = JSON.Parse(json_string);

        // Merge UserSettings into DefaultSettings
        foreach ( var rootElement in UserSettings)
        {
            switch (rootElement.Key)
            {
                case "ClockType":
                    AirSimSettings.GetSettings().ClockType = rootElement.Value;
                    break;
                case "ClockSpeed":
                    AirSimSettings.GetSettings().ClockSpeed = rootElement.Value.AsDouble;
                    break;
                case "Recording":
                    foreach (var sub1 in UserSettings["Recording"])
                    {
                        switch (sub1.Key)
                        {
                            case "RecordOnMove":
                                AirSimSettings.GetSettings().Recording.RecordOnMove = sub1.Value.AsBool;
                                break;
                            case "RecordInterval":
                                AirSimSettings.GetSettings().Recording.RecordInterval = sub1.Value.AsDouble;
                                break;
                            case "Cameras":
                                AirSimSettings.GetSettings().Recording.Cameras.Clear();
                                foreach (var sub2 in UserSettings["Recording"]["Cameras"])
                                {
                                    var camsettings = sub2.Value.AsArray;
                                    foreach (var cs in camsettings)
                                    {
                                        AirSimSettings.CamerasSettings NewRecordingCameraSetting = new AirSimSettings.CamerasSettings
                                        {
                                            CameraName = "0",
                                            ImageType = 0,
                                            PixelAsFloat = false,
                                            Compress = true
                                        };
                                        foreach (var cam in cs.Value)
                                        {
                                            switch (cam.Key)
                                            {
                                                case "CameraName":
                                                    NewRecordingCameraSetting.CameraName = cam.Value;
                                                    break;
                                                case "ImageType":
                                                    NewRecordingCameraSetting.ImageType = cam.Value.AsInt;
                                                    break;
                                                case "PixelAsFloat":
                                                    NewRecordingCameraSetting.PixelAsFloat = cam.Value.AsBool;
                                                    break;
                                                case "Compress":
                                                    NewRecordingCameraSetting.Compress = cam.Value.AsBool;
                                                    break;
                                            }
                                        }
                                        AirSimSettings.GetSettings().Recording.Cameras.Add(NewRecordingCameraSetting);
                                    }
                                }
                                break; // Cameras
                        }
                    }
                    break; // Recording
}

Is this the correct way if one wanted to first parse the root elements of the JSON data structure. And then also do subsequent parsing of sub elements? A little example of this would be helpful. :)

Bunny83 commented 5 years ago

Why do you actually iterate through all values? This makes sense for the camera array, but all other settings could be read directly.

var UserSettings = JSON.Parse(json_string);
var settings = AirSimSettings.GetSettings();

settings.ClockType = UserSettings["ClockType"];
settings.ClockSpeed = UserSettings["ClockSpeed"];

var recordingNode = UserSettings["Recording"];
settings.Recording.RecordOnMove = recordingNode["RecordOnMove"];
settings.Recording.RecordInterval = recordingNode["RecordInterval"];

settings.Recording.Cameras.Clear();
foreach (var camNode in recordingNode["Cameras"])
{
    var camSettings = new AirSimSettings.CamerasSettings
    {
        CameraName = camNode["CameraName"],
        ImageType = camNode["ImageType"],
        PixelAsFloat = camNode["PixelAsFloat"],
        Compress = camNode["Compress"]
    };
    settings.Recording.Cameras.Add(camSettings);
}

Note that the first line:

var UserSettings = JSON.Parse(json_string);

already parses the whole json text into nodes. You can then freely access any information. Such confusing foreach loops just make the performance worse. Switch cases with string values can't be implemented as a jump table as it is done for enums or ints. Switch cases of moderate length will effectively translate to an if-else chain. A JSONObject uses a Dictionary internally which can do a string lookup actually faster and the resulting code is easier to read.

ps: this line in your original code: var camsettings = sub2.Value.AsArray; doesn't seem to make much sense since a single camera setting seems to be an object, not an array.

Note that the JSONNode class now has implicit conversion operators and the compiler will automatically use the right conversion. So if you assign a JSONNode value to an int variable it will automatically use "AsInt".

Bunny83 commented 5 years ago

Note my comment assumed a json file like this:

{
    "ClockType" : "yourClockType",
    "ClockSpeed" : 1.23,
    "Recording" : {
        "RecordOnMove" : true,
        "RecordInterval" : 0.25,
        "Cameras" : [
            {
                "CameraName" : "Cam 1",
                "ImageType" : 2,
                "PixelAsFloat" : false,
                "Compress" : true
            },
            {
                "CameraName" : "Cam 2",
                "ImageType" : 2,
                "PixelAsFloat" : false,
                "Compress" : true
            },
        ]
    }
}
jasonhbartlett commented 5 years ago

Ahhh... That make sense. Sorry for the missing that obvious usage. Thanks so much for your quick response. As I said this is going to save me a lot of time and I really appreciate it!

jasonhbartlett commented 5 years ago

Hi, Regarding:

image

Should it be changed to this?

foreach (var camNode in recordingNode["Cameras"].Values)

??

Also, do you see any problem with using the null-coalescing operator ?? to assign a default value if the user did not specify the attribute?

Bunny83 commented 5 years ago

My bad, it should be either

foreach (JSONNode camNode in recordingNode["Cameras"])

or when you used the orininal code, you would have to use

foreach (var camNode in recordingNode["Cameras"])
{
    CameraName = camNode.Value["CameraName"],

The normal enumerator uses KeyValuePairs. The framework comes with an implicit conversion from KeyValuePair(string, JSONNode) to JSONNode. So when we declare camNode explicitly as JSONNode it will be converted automatically.


However using .Children does work as well. Though the Children enumerator will allocate memory as it's a classic IEnumerable


The null-coalescing would not work as the framework would return a lazy creator object in case a value doesn't exist. The lazy creator implements a custom == operator which would return true if compared against null. However the null-coalescing operator does not use the == operator for the null check. It directly compares the reference against null which will never be the case.


I quickly added a "HasKey" and a GetValueOrDefault method to simplify reading conditionally. So you can do something like this:

CameraName = camNode.GetValueOrDefault("CameraName", "defaultName")

Note that the default value is of type JSONNode. Since all supported types are implicitly converted to JSONNode you can directly pass int, string, float, bool, ... values. Just keep in mind that the "defaultName" will actually result in new JSONString("defaultName").

jasonhbartlett commented 5 years ago

Friend, I don't quite follow what you mean in that last paragraph, but you are brilliant and it works brilliantly! Thanks so much! Here is what I can do now:

if (UserSettings.HasKey("Recording"))
        {
            var recordingNode = UserSettings["Recording"];
            Settings.Recording.RecordOnMove = recordingNode.GetValueOrDefault("RecordOnMove", false);
            Settings.Recording.RecordInterval = recordingNode.GetValueOrDefault("RecordInterval", 0.05);
            if (recordingNode.HasKey("Cameras"))
            {
                Settings.Recording.Cameras.Clear();
                foreach (JSONNode camNode in recordingNode["Cameras"])
                {
                    var NewCamerasSettings = new AirSimSettings.CamerasSettings
                    {
                        CameraName = camNode.GetValueOrDefault("CameraName", "0"),
                        ImageType = camNode.GetValueOrDefault("ImageType", 0),
                        PixelAsFloat = camNode.GetValueOrDefault("PixelAsFloat", false),
                        Compress = camNode.GetValueOrDefault("Compress", true)
                    };
                    Settings.Recording.Cameras.Add(NewCamerasSettings);
                }
            }
        }

This lets me smoothly fill in the blanks with default values of a user settings.json file with potentially lots of missing attributes. Thanks so much!

jasonhbartlett commented 4 years ago

Thanks again for your help! Closing this issue.