0
\$\begingroup\$

Presentation:

I am trying to write classes to store data from a database.

The data consists in objects, let's call them "Areas". Each area has a name and possibly 'child' areas:

public class Area {
    public string Name;
    public List<Area> SubAreas = new List<Area>();
}

The program reads data from the database (out of the scope of this review), and creates a new Data object to store this data.

The data is returned as a 2D array of stings: [ [areaName, parentAreaName], ...]

With the following assumptions:

  • The parent of a root-level area is String.Empty
  • The parent item is before (in the array) any child item (-> no recurrence needed).

Code:

public static void Main()
    {
        //This comes from the DB
        string[,] areas = new string[4,2] {{"1",""}, {"11","1"},{"12","1"},{"2",""}};   

        new Data(areas);
    }

public class Data {
    public List<Area> Areas = new List<Area> ();

    public Data(string[,] areasArray)
    {
        for (int i = 0; i < areasArray.GetLength(0); i++) {
            Area loopSubArea;
            if (areasArray[i,1] != "") {
                loopSubArea = Areas.Where(a => a.Name == areasArray[i,1]).First();
                loopSubArea.SubAreas.Add(new Area() {Name = areasArray[i,0]});
            } else {
                Areas.Add(new Area() {Name = areasArray[i,0]});
            }
        }
    }
}

The full working code is also available in a Fiddle

My concerns:

  • I initialize the lists during the object creation, meaning that I have possibly a lot of empty Lists around. Is that an issue?
  • I feel like the loop part could be improved, but I have not been able to think of anything better
  • I am a beginner in C#, any general advice is of course welcome.
\$\endgroup\$
1
\$\begingroup\$

regarding your concerns.

1) I do not believe this initialization is a problem but if later on it will be a problem, you can always change it to lazy initialization.

2,3) I just paste a similar code that I find more readable and improves your loop when you want to find the parent area.

    public class Data
    {
        public List<Area> Areas = new List<Area>();

        public Data(string[,] areasArray)
        {
            for (int i = 0; i < areasArray.GetLength(0); i++)
            {
                var areaName = areasArray[i, 0];
                var parentAreaName = areasArray[i, 1];

                if (string.IsNullOrEmpty(parentAreaName))
                {
                    Areas.Add(new Area() { Name = areaName });
                }
                else
                {
                    var parentArea = Areas.First(a => a.Name == parentAreaName);
                    parentArea.SubAreas.Add(new Area() { Name = areaName });
                }
            }
        }
    }

You can probably still improve the loop but I guess you will lost readability, so I would leave it like this unless you face a real performance issue.

Regards

\$\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.