10
\$\begingroup\$

I am wondering what is the best way to map array values to properties in a class. Consider the following sample array describing information for an airport:

        [0] "6523"  string
        [1] "00A"   string
        [2] "heliport"  string
        [3] "Total Rf Heliport" string
        [4] "40.07080078125"    string
        [5] "-74.9336013793945" string
        [6] "11"    string
        [7] "NA"    string
        [8] "US"    string
        [9] "US-PA" string
        [10]    "Bensalem"  string
        [11]    "no"    string
        [12]    "00A"   string
        [13]    ""  string
        [14]    "00A"   string
        [15]    ""  string
        [16]    ""  string
        [17]    ""  string

I have the following class:

    public class Airport
    {
        public Airport(string[] data)
        {
            Action<string>[] PropertyMappings =
            {
                x=>this.Id=x,
                x=>this.Ident=x,
                x=>this.Type=x,
                x=>this.Name=x,
                x=>this.Latitude=x,
                x=>this.Longtitude=x,
                x=>this.Elevation=x,
                x=>this.Continent=x,
                x=>this.CountryIso=x,
                x=>this.RegionIso=x,
                x=>this.Municipality=x,
                x=>this.ScheduledService=x,
                x=>this.GPSCode=x,
                x=>this.DataCode=x,
                x=>this.LocalCode=x,
                x=>this.HomeLink=x,
                x=>this.WikipediaLink=x,
                x=>this.Keywords=x
            };
            for(int i=0;i<data.Count();i++)
            {
                PropertyMappings[i](data[i]);
            }
        }
        public string Id { get; set; }
        public string Ident { get; set; }
        public string Type { get; set; }
        public string Name { get; set; }
        public string Latitude { get; set; }
        public string Longtitude { get; set; }
        public string Elevation { get; set; }
        public string Continent { get; set; }
        public string CountryIso { get; set; }
        public string RegionIso { get; set; }
        public string Municipality { get; set; }
        public string ScheduledService { get; set; }
        public string GPSCode { get; set; }
        public string DataCode { get; set; }
        public string LocalCode { get; set; }
        public string HomeLink { get; set; }
        public string WikipediaLink { get; set; }
        public string Keywords { get; set; }
    }
}

And I call it like this:

Airport airport = new Airport(data);

Do you think this is a good way to do the mapping from the elements of the array to the properties of the class or is there a better way. I couldn't really find anything online. Obviously I haven't done all of the safety checks etc. This is just a small experiment.

\$\endgroup\$
  • \$\begingroup\$ Will the array always have 1 element for each property? That is to say, you don't need to support arrays with only a few of the properties. \$\endgroup\$ – RobH Jan 8 '15 at 10:17
  • \$\begingroup\$ Yes always exactly one value for each property \$\endgroup\$ – Phoenix Jan 8 '15 at 10:41
7
\$\begingroup\$

Basically this looks good but it can be improved.

  • Also, you won't find any hints about casing variable names which are local to a method in the naming guidelines you should consider to use camelCase casing.

  • the mapping of the properties should be extracted to a separate method and be called from the constructor. In this way you can reuse it and inside of the constructor the amount of code is reduced.

  • let your conditions and variables breathe. This for(int i=0;i<data.Count();i++) would look much nicer like for (int i = 0; i < data.Count(); i++)

  • instead of i<data.Count() you should use the Length property of the array. But to be on the safe side, you should use something like

    int minLength = Math.Min(data.Length, PropertyMappings.Length);  
    for (int i = 0; i < minLength; i++)  
    
  • some would say you should use var i=0; instead int i=0;

  • Because the Airport's properties won't be changed, the setters should be protected.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ The C# goldie is within sight ;-) \$\endgroup\$ – janos Jun 16 '15 at 7:40
8
\$\begingroup\$

As you said that you will always have the right number of elements in the array, I would suggest the following:

public class Airport
{
    public string Id { get; set; }
    public string Ident { get; set; }
    public string Type { get; set; }
    public string Name { get; set; }
    public string Latitude { get; set; }
    public string Longtitude { get; set; }
    public string Elevation { get; set; }
    public string Continent { get; set; }
    public string CountryIso { get; set; }
    public string RegionIso { get; set; }
    public string Municipality { get; set; }
    public string ScheduledService { get; set; }
    public string GPSCode { get; set; }
    public string DataCode { get; set; }
    public string LocalCode { get; set; }
    public string HomeLink { get; set; }
    public string WikipediaLink { get; set; }
    public string Keywords { get; set; }

    public static Airport CreateFromData(string[] data)
    {
        if (data.Length != 18) {
            throw new ArgumentException("...");
        }
        return new Airport
                   {
                       Id = data[0],
                       Ident = data[1],
                       Type = data[2],
                       Name = data[3],
                       Latitude = data[4],
                       Longtitude = data[5],
                       Elevation = data[6],
                       Continent = data[7],
                       CountryIso = data[8],
                       RegionIso = data[9],
                       Municipality = data[10],
                       ScheduledService = data[11],
                       GPSCode = data[12],
                       DataCode = data[13],
                       LocalCode = data[14],
                       HomeLink = data[15],
                       WikipediaLink = data[16],
                       Keywords = data[17]
                   };
    }        
}

Depending on usage I would suggest that you alter the accessibility of the setters on the properties and consider making the default constructor protected (if you only want to create via a data array).

Why am I making this suggestion?

  1. Keeps it simple
  2. It's about 30 times faster than your method
  3. You can view the code as a schema for the data array. I.e. Type is the third element

Edit

As RufusL notes in the comments, it would be a good idea to add a null check for the data array:

public static Airport CreateFromData(string[] data)
{
    if (data == null) {
        throw new ArgumentNullException("data");
    }
    if (data.Length != 18)
    {
        ...

Further to my comment about my proposed solution being faster - it's not because your code is slow in any way. If you want a dictionary of mappings of property -> index in data array your code will be slower but that probably wont matter.

You could just move your array of property mappings to a field. Just change the signature to Action<Airport,string>[] you'd then populate it like:

Action<Airport, string>[] PropertyMappings = 
{
     (a, s) => a.Id = s,
     // ...
}

public static Airport CreateFromData(string[] data)
{
    // Guards omitted.
    var result = new Airport();
    for (var i = 0; i < data.Length; i++) 
    {
        PropertyMappings(result, data[i]);
    }
    return result;
}

(all of the code in the edit has just been typed directly in the browser, if it doesn't compile, hopefully it should be close enough to correct.)

|improve this answer|||||
\$\endgroup\$
  • 2
    \$\begingroup\$ I like this. I'm not sure how OP's code would react to an array with only 10 elements, but I know exactly how this would react to it. \$\endgroup\$ – RubberDuck Jan 8 '15 at 11:37
  • \$\begingroup\$ I can see your point. I wanted to be able to select what index each property corresponds to and store that data in some kind of data structure. Now that I look at what I've done yesterday, it seems way too complicated for something as simple as that. Still do you have any idea how I can achieve that? I want to have something like a Dictionary<string(or whatever else),int> and store the name of the property or some reference to it as the key and the index of the data for that property in the value. \$\endgroup\$ – Phoenix Jan 8 '15 at 16:00
  • \$\begingroup\$ Also could you please explain why my initial solution is so slow? \$\endgroup\$ – Phoenix Jan 8 '15 at 16:01
  • 1
    \$\begingroup\$ Your initial solution isn't slow per se, it's just that adding a layer of indirection through your Actions is a lot slower than trivially newing up the object with direct array access. As to your other question, reflection and an attribute would be one option. I don't have time right now but I'll update either tonight or tomorrow with an example of what I'm thinking. The speed *shouldn't matter in real world workloads - it was about 750,000 creations per second. \$\endgroup\$ – RobH Jan 8 '15 at 16:27
  • \$\begingroup\$ Would recommend adding a conditional check for data == null before you try to access the .Length property. Either a separate if, or added to your existing one: if (data == null || data.Length != 18) \$\endgroup\$ – Rufus L Jan 9 '15 at 19:25

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.