HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Populating values into a C# object

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
valuesintoobjectpopulating

Problem

I have written a function to populate values into a class whose structure is as given below

public class Country
    {
        public string Name { get; set; }
        public string Continent { get; set; }
        public string Zone { get; set; }
        public India IndiaInfo { get; set; }

    }

   public class India
    {
        public List StateInfo{get; set;}
    }

    public class State
    {
        public string StateName { get; set; }
        public string StateCode { get; set; }
    }


The values are populated into the class object as shown below

public void PopulateCountry(string name, string Asia , string GMT , string AbcState, string RichState)
    {
        Country objCountry = new Country();
        objCountry.Continent = Asia;
        objCountry.Name = name;
        objCountry.Zone = GMT;

        **India objIndia = new India();
        objIndia.StateInfo = new List;
        State objState = new State();
        objState.StateCode = AbcState;
        objState.StateName = RichState;
        objIndia.StateInfo.Add(objState);
        objCountry.IndiaInfo = objIndia;**

    }


In the above method to populate the object of class Country I have taken a brute force approach and put the values in the class and the code is giving results too. However this code is not following some of the best practices and can be further enhanced /rewritten especially the one in Bold. Can someone point out the things/features/practices of C# I am missing.

Edit# 1 Magus pointed out some of the conventions i was not following Regarding this particular method, PopulateCountry 1) can we have better methods to instantiate the state object/india object , taking into account their relationship as per class structure 2) the last line of code where i have directly assigned the object,is that fine and according to standards?

Solution

A few things straight away:

You have a class called India which really just holds a collection of States. I would suggest not using that class at all, instead creating a List States instead.

Info as a suffix is fairly meaningless, particularly when the object is a collection.

You also do not need to put obj in front of objects you create. In C#, everything is an object, so that means absolutely nothing.

In addition, the names you've chosen for parameters are particularly bad. Asia might be a good value, but continent is a far better variable name.

Last of all, you may want to use object initializers:

public class Country
{
    public string Name { get; set; }
    public string Continent { get; set; }
    public string Zone { get; set; }
    public List Country { get; set; }
}

public void PopulateCountry(string name, string continent, string timeZone, string stateCode, string stateName)
{
    var state = new State
    {
        StateCode = stateCode,
        StateName = stateName
    }
    var country = new Country
    {
        Continent = continent,
        Name = name,
        Zone = timeZone,
        Country = new List { state }
    }
}


This isn't everything you can do. It may make more sense to make a Country class if you plan on adding other things to it. This at the very least makes your code simpler, though.

Alternatively, you could assign a country like this, anywhere:

var country = new Country
{
    Continent = continent,
    Name = name,
    Zone = timeZone,
    Country = new List
    {
       new State
       {
           Name = stateName1,
           Code = stateCode1
       },
       new State
       {
           Name = stateName2,
           Code = stateCode2
       },
    }
}


Or even:

var country = new Country
{
    Continent = continent,
    Name = name,
    Zone = timeZone
}
country.Country = names.Zip(codes, (x, y) => new State { Name = x, Code = y };


Or something close to it, assuming the two are in the correct order. You have many options.

Code Snippets

public class Country
{
    public string Name { get; set; }
    public string Continent { get; set; }
    public string Zone { get; set; }
    public List<State> Country { get; set; }
}

public void PopulateCountry(string name, string continent, string timeZone, string stateCode, string stateName)
{
    var state = new State
    {
        StateCode = stateCode,
        StateName = stateName
    }
    var country = new Country
    {
        Continent = continent,
        Name = name,
        Zone = timeZone,
        Country = new List<State> { state }
    }
}
var country = new Country
{
    Continent = continent,
    Name = name,
    Zone = timeZone,
    Country = new List<State>
    {
       new State
       {
           Name = stateName1,
           Code = stateCode1
       },
       new State
       {
           Name = stateName2,
           Code = stateCode2
       },
    }
}
var country = new Country
{
    Continent = continent,
    Name = name,
    Zone = timeZone
}
country.Country = names.Zip(codes, (x, y) => new State { Name = x, Code = y };

Context

StackExchange Code Review Q#69533, answer score: 6

Revisions (0)

No revisions yet.