1
\$\begingroup\$

I have a json file - WorkflowNames.json with contents as -

["ImportOrder", "ValidateOrder", "ProcessOrder", "ArchiveOrder"]

My C# code to read this json file is -

private Dictionary<string, bool> GetWorkflowNames()
{
  var appDataFolder = ConfigurationMgr.GetAppPath("App_Data");
  var jsonFilePath = Path.Combine(appDataFolder, "WorkflowNames.json");
  JArray workflowNames = (JArray)JsonConvert.DeserializeObject(System.IO.File.ReadAllText(jsonFilePath));

  var workflowNamesDictionary = new Dictionary<string, bool>();
  foreach (JToken workflowName in workflowNames)
  {
    workflowNamesDictionary.Add(workflowName.ToString(), false);
  }

  return workflowNamesDictionary;
}

My code runs fine. But because this is my first attempt to read data from a json file, I am not sure about these -

  1. As a json standard, any data in json file should be represented as key value pair. If I have just an array to read from json file, is it okay to declare as above?
  2. Is there any better way, can I improve/simplify above C# code?

Thanks a lot!

\$\endgroup\$
3
\$\begingroup\$

Json.Net has support for collections built-in. You should be able to use the generic version of DeserializeObject like this:

private Dictionary<string, bool> GetWorkflowNames()
{
    var appDataFolder = ConfigurationMgr.GetAppPath("App_Data");
    var jsonFilePath = Path.Combine(appDataFolder, "WorkflowNames.json");
    var jsonString = System.IO.File.ReadAllText(jsonFilePath);
    var workflowNames = JsonConvert.DeserializeObject<List<string>>(jsonString);
    return workflowNames.ToDictionary(name => name, _ => false);
}

There are still a few things I would clean up in your code:

Add a using for System.IO and use File without the namespace. You could also use a static using like this using static System.IO.File; and access ReadAllText directly but I generally dislike this unless the class is very focussed and heavily using File.

You'll notice that I introduced a local variable jsonString as I don't like hiding the file access in the call to deserialize the json. I try to stick rigidly to one thing happening per line. Simple and clear code is best when you want to maintain it 2 years later.

I'd also recommend using .ToDictionary as it shortens the code and makes it very obvious what you're doing. Having said that, I would have this return an IEnumerable<string> instead of a dictionary. It's not clear why this returns a dictionary and GetWorkflowNames sounds like it would be perfectly reasonable to return the list.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks RobH, really appreciate your detailed review. I loved your statement "I try to stick rigidly to one thing happening per line." The reason for returning Dictionary object is to add to another dictionary (existing legacy code) in the calling method. \$\endgroup\$ – iniki Jul 10 at 8:42

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.