Mdthbs

I have two base classes: City and Building:

public class City

{

    public string Name { get; set; }

    public double Area { get; set; }

}



public class Building

{

    public string Name { get; set; }

    public double Area { get; set; }

    public int Stories { get; set; }

}

With two classes that inherit from Building, with their own Id property:

public class MyBuilding : Building

{

    public int MyId { get; set; }

}



public class HisBuilding : Building

{

    public int HisId { get; set; }

}

And two classes that inherit from City, with their own Id and IEnumerable<Building> properties:

public class MyCity : City

{

    public int MyId { get; set; }

    public IEnumerable<MyBuilding> Buildings { get; set; }

}



public class HisCity : City

{

    public int HisId { get; set; }

    public IEnumerable<HisBuilding> Buildings { get; set; }

}

Below is a method I've created that returns a KeyValuePair<City, Building> using generics:

public static KeyValuePair<TCity, IEnumerable<TBuilding>> GetData<TCity, TBuilding>()

    where TCity : City, new()

    where TBuilding : Building, new()

{

    TCity city = new TCity();

    IEnumerable<TBuilding> buildings = new List<TBuilding>();

    return new KeyValuePair<TCity, IEnumerable<TBuilding>>(city, buildings);

}

And in my Main method I've logic that instantiates a specific City object based on a user's input.

int input = -1;

string strInput = Console.ReadLine();

int.TryParse(strInput, out input);

string json = string.Empty;

switch (input)

{

    case 1:

        KeyValuePair<MyCity, IEnumerable<MyBuilding>> myCity

            = GetData<MyCity, MyBuilding>();

        MyCity my = myCity.Key;

        my.MyId = 1;

        my.Buildings = myCity.Value;

        json = JsonConvert.SerializeObject(my, Formatting.Indented);

        break;



    default:

        KeyValuePair<HisCity, IEnumerable<HisBuilding>> hisCity

            = GetData<HisCity, HisBuilding>();

        HisCity his = hisCity.Key;

        his.HisId = 2;

        his.Buildings = hisCity.Value;

        json = JsonConvert.SerializeObject(his, Formatting.Indented);

        break;

}

Console.WriteLine(json);

Console.ReadLine();

The above generic method is just something I cobbled together in the last half hour, but I'm wondering if there is a more efficient way of creating these custom objects of mine, especially when each City has its own, different IEnumerable<Building> property.

EDIT

Here is the code I have for building the IEnumerable inside my GetData method:

public static IEnumerable<TBuilding> GetBuildingData<TBuilding>() 

    where TBuilding : Building, new()

{

    DataTable table;

    string myConnectionString = @"Data Source=(LocalDb)MSSQLLocalDb;Initial Catalog=Test;Integrated Security=True;";

    List<TBuilding> buildings = new List<TBuilding>();

    using (SqlConnection connection = new SqlConnection(myConnectionString))

    {

        using (SqlCommand command = new SqlCommand("[dbo].[GetListData]", connection) { CommandType = CommandType.StoredProcedure })

        {

            connection.Open();

            using (SqlDataAdapter adapter = new SqlDataAdapter(command))

            {

                table = new DataTable();

                adapter.Fill(table);

            }

            connection.Close();

        }

    }

    foreach (DataRow row in table.Rows)

    {

        TBuilding building = new TBuilding();

        building.Area = Convert.ToDouble(row["Area"]);

        building.Name = row["Name"].ToString();

        building.Stories = Convert.ToInt32(row["Stories"]);

        buildings.Add(building);

    }

    return buildings;

}

I will focus on..

Creating custom objects with custom properties using generics

Which you do in this method..

public static KeyValuePair<TCity, IEnumerable<TBuilding>> GetData<TCity, TBuilding>()

    where TCity : City, new()

    where TBuilding : Building, new()

{

    TCity city = new TCity();

    IEnumerable<TBuilding> buildings = new List<TBuilding>();

    return new KeyValuePair<TCity, IEnumerable<TBuilding>>(city, buildings);

}

• A KeyValuePair is archaic, you can use a ValueTuple nowadays


• 
  TBuilding does not require the new() constraint in this method


• The choice for IEnumerable over IList is questionable, since you retrieve the items later on and have no way of adding them to an IEnumerable. So why create the IEnumerable in this method?


• The name should reflect what you are doing. CreateCity seems more appropriate.

I would refactor this method..

public static (TCity city, IList<TBuilding> buildings) CreateCity<TCity, TBuilding>()

    where TCity : City, new()

    where TBuilding : Building

{

    var city = new TCity();

    var buildings = new List<TBuilding>();

    return (city, buildings);

}

EDIT:

Thinking about the refactored method. This doesn't even need to be a method anymore. It doesn't do anything but create objects for the types you specify.

What I would really do is throw out this ancient way of ORM, and use an existing API (EF, NHibernate, Dapper).

public class MyBuilding : Building

{

    public int MyId { get; set; }

}



public class HisBuilding : Building

{

    public int HisId { get; set; }

}

Why do you have the id as properties of the subclasses, it's a candidate for a base class member:

public class Building

{

  public int Id { get; set; }

If the subclasses must have specialized names for their Id property, then provide that as:

public class MyBuilding : Building

{

    public int MyId { get { return Id; } set { Id = value; } }

}

but that is a rather odd concept that you should avoid if possible.

The same holds for Cities.

public class MyCity : City

{

    public int MyId { get; set; }

    public IEnumerable<MyBuilding> Buildings { get; set; }

}

Normally you would have a materialized data set for buildings instead of an IEnumerable<T> - unless you're creating the instances lazily/dynamically. You could maybe consider using a IReadonlyList<T> as type, if you don't want it to be modifiable or else just a IList<T>

public static KeyValuePair<TCity, IEnumerable<TBuilding>> GetData<TCity, TBuilding>()

    where TCity : City, new()

    where TBuilding : Building, new()

{

    TCity city = new TCity();

    IEnumerable<TBuilding> buildings = new List<TBuilding>();

    return new KeyValuePair<TCity, IEnumerable<TBuilding>>(city, buildings);

}

If you changed the City base class to a generic like:

public class City<TBuilding> where TBuilding: Building

{

  public IList<TBuilding> Buildings { get; set; }

}

and dropped the buildings on the specialized cities, then you could return just the city from GetData, because you can initialize the Buildings property inside GetData, which I would rename to CreateCity()

public static TCity CreateCity<TCity, TBuilding>()

where TCity : City<TBuilding>, new()

where TBuilding : Building, new()

{

  TCity city = new TCity();

  city.Buildings = new List<TBuilding>();

  return city;

}

Ideally you could have a common baseclass for City and Building, because they share some significant properties like Id, Area and Name:

public abstract class AreaObject

{

  public int Id { get; set; }

  public string Name { get; set; }

  public double Area { get; set; }

}

A complete refactoring of your data model could then be:

public abstract class AreaObject

{

  public int Id { get; set; }

  public string Name { get; set; }

  public double Area { get; set; }

}



// For convenience a city base class without the generic type parameter:

public abstract class City : AreaObject

{

  public abstract IEnumerable<Building> GetBuildings();

}



public class City<TBuilding> : City where TBuilding : Building

{

  public IList<TBuilding> Buildings { get; set; }



  public override IEnumerable<Building> GetBuildings()

  {

    return Buildings;

  }

}



public class Building : AreaObject

{

  public int Stories { get; set; }

}



public class MyBuilding : Building

{

  public int MyId { get { return Id; } set { Id = value; } }

}



public class HisBuilding : Building

{

  public int HisId { get { return Id; } set { Id = value; } }

}



public class MyCity : City<MyBuilding>

{

  public int MyId { get { return Id; } set { Id = value; } }

}



public class HisCity : City<HisBuilding>

{

  public int HisId { get { return Id; } set { Id = value; } }

}

It's a little odd to have two different sub cities having specialized buildings instead of just Buildings, but you may have reasons for that? (What if MyCity buys a building from HisCity can it then change from His to My?)

Here is the code I have for building the IEnumerable inside my
GetData method:

public static IEnumerable<TBuilding> GetBuildingData<TBuilding>()...

If that is going to be used inside GetData() shouldn't it then take a city id as argument in order to minimize the query? If so you can change the GetData to:

public static TCity CreateCity<TCity, TBuilding>(int id)

where TCity : City<TBuilding>, new()

where TBuilding : Building, new()

{

  TCity city = new TCity();

  city.Buildings = GetBuildingData<TBuilding>(id);

  city.Id = id;

  return city;

}

And you'll then have to modify your database query to only return the buildings for that city:

public static IList<TBuilding> GetBuildingData<TBuilding>(int cityId)

where TBuilding : Building, new() {...}

All in all this could simplify your use case to:

  int cityId = -1;

  string strInput = Console.ReadLine();

  int.TryParse(strInput, out cityId);

  if (cityId > 0)

  {

    City city = null;



    switch (cityId)

    {

      case 1:

        city = CreateCity<MyCity, MyBuilding>(cityId);

        break;

      case 2:

        city = CreateCity<HisCity, HisBuilding>(cityId);

        break;

      default:

        throw new InvalidOperationException("Invalid Id");

    }



    string json = JsonConvert.SerializeObject(city, Formatting.Indented);

    Console.WriteLine(json);

    Console.ReadLine();

  }