CODEWITHSUNDEEP
c#loopsforeach
dokyalashot
dokyalashot

Reputation: 115

Simplify multiple (non nested)foreach loops in c#

I have a JSON object:

"Cars": {
 "Honda": {
   "CRV": [

     { "index": 1, "Color": "Black" },
     { "index": 2, "Color": "White" }

    ],
    "Civic": [

     { "index": 1, "Color": "Blue" }

    ]
   },
  "Toyota": {
   "Corolla": [

     { "index": 1, "Color": "Black" }

    ],
    "Camry": [

     { "index": 1, "Color": "Blue" }

    ]
   },
  "GM": {
   "Chevrolet": {

     "Cruze": [

       { "index": 1, "Color": "Blue" }

    ]

    }
 }

I have to store this information in a table. Right now I am parsing each List in a separate for each loop like this:

 foreach (CarInfo info in Cars.Honda.CRV)
  {
                //storing in table
  }
  foreach (CarInfo info in Cars.Honda.Civic)
  {
                //storing in table
  }
 foreach (CarInfo info in Cars.Toyota.Corolla)
  {
                //storing in table
  }

and so on. Is there a way I could optimize this code? (Given is just an example, the actual JSON is more extensive and had made me write way too many foreach loops)

The method for storing the info in table is different for each Car make.

Upvotes: 2

Views: 531

Answers (3)

user5429970
user5429970

Reputation: 86

Since you have no control over the input JSON, here's an approach similar to my last recommendation that parses up the incoming JSON, including support for GM submodels.

static void Main(string[] args)
{
    // parse the JSON results of the API Call
    JObject apiResult = JObject.Parse(File.ReadAllText("JsonBlock.json"));
    // iterate through the models
    foreach (var model in apiResult["Cars"].Children<JProperty>().Select(i => i.Name))
        // GM cars have a submodel
        if(model.Equals("GM",StringComparison.CurrentCultureIgnoreCase)) 
        {
            foreach (var submodel in apiResult["Cars"][model].Children<JProperty>().Select(i => i.Name))
                AddCar(submodel,apiResult["Cars"][model][submodel]);
        } 
        else
            AddCar(model,apiResult["Cars"][model]);            
}

static void AddCar(string model, JToken cars)
{
    switch(model)
    {
        case "Honda":                    
            // do honda things
            break;
        case "Toyota":
            // do toyota things
            break;
        case "Chevrolet":
            // do chevy things
            break;
        default:
            throw new NotImplementedException();
    }            
}

Upvotes: 1

Jawad
Jawad

Reputation: 11364

How about something like this... Recursive Approach since your classes dont line up with Cars/Make/Model, you have Cars//Make/Model.

  1. Parse the Json to JObject
  2. Create a dictionary for each Make so you can save them separately via your distinct process
    public static Dictionary<string, List<CarInfo>> GetCarInfo (JObject jObject)
    {
        Dictionary<string, List<CarInfo>> list = new Dictionary<string, List<CarInfo>>();
        foreach (var property in jObject.Properties())
        {
            var thisProp = jObject[property.Name];
            if (thisProp.Type.ToString().Equals("Array"))
                    list.Add(property.Name, JsonConvert.DeserializeObject<List<CarInfo>>(thisProp.ToString()));
            else
                GetCarInfo((JObject)jObject[property.Name]).ToList().ForEach(x => list.Add(x.Key, x.Value));

            }
        return list;
    }

    public static void SaveData(string key, List<CarInfo> value)
    {
        switch (key.ToUpper())
        {
            case "CRV":
                // save it the CRV way
                break;

            case "COROLLA":
                // save it this way
                break;

            default:
                throw new ApplicationException("Unable to figure out which car it is..");
        }
    }

Usage in Main

    var cars = JObject.Parse(json);
    Dictionary<string, List<CarInfo>> carInfos = GetCarInfo((JObject)cars["Cars"]);
    foreach (var carInfo in carInfos)
        SaveData(carInfo.Key, carInfo.Value);

Note

I noticed that your json object doesnt really follow the same standard. Your GM has two parent companies when Honda, Toyota have one.

Suggestion

Only thing I would suggest to make the code look and perform better is to update the way you make a call to update / add to DB. Currently, you have one call per type which is what defines your foreach loops. If you can make one generic method to save "CarInfo" with another argument that defines "CRV", or "CORROLLA", it would make things Much easier for you.

Upvotes: 0

user5429970
user5429970

Reputation: 86

Is the source data format under your control?

  • As 3Dave mentioned, you'll be much happier if the car make and model are values instead of keys.
  • I might add that you currently have a single "Cars" object. You might find this much easier to manage if you have an array of "Car" objects, each of which has their various attributes internal.
{ "Cars": [
  { "make": "honda",
    "model": "CRV",
    "attributes": ["index":1,"color":"blue"]
  },
  { "make": "honda",
    "model": "CRV",
    "attributes": ["index":2,"color":"white"]
  },
  { "make": "honda",
    "model": "Civic",
    "attributes": ["index":1,"color":"black"]
  }
]}

This makes it trivial to collapse your iteration down to a single foreach loop, then handle the different method for storing in a table within a subroutine:

void AddAllCars()
{
  foreach (var car in Cars)
    AddCar(car);
}

void AddCar(CarInfo car)
{
    switch(car.Make)
    {
      case "honda":
        // honda-specific stuff
        break;
      case "toyota":
        // toyota-specific stuff
        break;
    }
}

(Note that even if you can't reformat into an array of Cars, you can still make big improvements by changing make and model to values instead of keys)

Upvotes: 1

Related Questions