Skip to main content
Code Review

Return to Answer

update with a more conventional class hierarchy.
Source Link
user73941
user73941

Update

As mentioned a couple of times in the above, the data model and class structure for especially City is rather unusual. A more conventional structure could be something like:

public abstract class AreaObject
{
 public int Id { get; set; }
 public string Name { get; set; }
 public double Area { get; set; }
}
public abstract class City : AreaObject
{
 public IList<Building> Buildings { get; set; }
}
public class MyCity : City
{
 // Uncomment if you really need this: public IList<MyBuilding> MyBuildings => Buildings.Where(b => b is MyBuilding).Cast<MyBuilding>().ToList();
}
public class HisCity : City
{
 // Uncomment if you really need this: public IList<HisBuilding> HisBuildings => Buildings.Where(b => b is HisBuilding).Cast<HisBuilding>().ToList();
}
public class Building : AreaObject
{
 public int Stories { get; set; }
}
public class MyBuilding : Building
{
}
public class HisBuilding : Building
{
}

As shown, City owns the Buildings property. The original concept of having specialized building properties per city sub class, doesn't make much sense in real life, because what defines a building: its relation to a city or its materials, functionality etc.? Can't two cities have a copy of the same building? And a MyCity should be able to Buy a building from HisCity etc. And a city should be able to own different types of building.

The factory method could then be simplified to this:

 public static TCity CreateCity<TCity>(int id)
 where TCity : City, new()
 {
 TCity city = new TCity();
 city.Buildings = GetBuildingData(id);
 city.Id = id;
 return city;
 }
 public static IList<Building> GetBuildingData(int cityId)
 {
 // The sql stuff...
 }

And the use case to:

 public static void Run()
 {
 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>(cityId);
 break;
 case 2:
 city = CreateCity<HisCity>(cityId);
 break;
 default:
 throw new InvalidOperationException("Invalid Id");
 }
 string json = JsonConvert.SerializeObject(city, Formatting.Indented);
 Console.WriteLine(json);
 Console.ReadLine();
 }
 }

Update

As mentioned a couple of times in the above, the data model and class structure for especially City is rather unusual. A more conventional structure could be something like:

public abstract class AreaObject
{
 public int Id { get; set; }
 public string Name { get; set; }
 public double Area { get; set; }
}
public abstract class City : AreaObject
{
 public IList<Building> Buildings { get; set; }
}
public class MyCity : City
{
 // Uncomment if you really need this: public IList<MyBuilding> MyBuildings => Buildings.Where(b => b is MyBuilding).Cast<MyBuilding>().ToList();
}
public class HisCity : City
{
 // Uncomment if you really need this: public IList<HisBuilding> HisBuildings => Buildings.Where(b => b is HisBuilding).Cast<HisBuilding>().ToList();
}
public class Building : AreaObject
{
 public int Stories { get; set; }
}
public class MyBuilding : Building
{
}
public class HisBuilding : Building
{
}

As shown, City owns the Buildings property. The original concept of having specialized building properties per city sub class, doesn't make much sense in real life, because what defines a building: its relation to a city or its materials, functionality etc.? Can't two cities have a copy of the same building? And a MyCity should be able to Buy a building from HisCity etc. And a city should be able to own different types of building.

The factory method could then be simplified to this:

 public static TCity CreateCity<TCity>(int id)
 where TCity : City, new()
 {
 TCity city = new TCity();
 city.Buildings = GetBuildingData(id);
 city.Id = id;
 return city;
 }
 public static IList<Building> GetBuildingData(int cityId)
 {
 // The sql stuff...
 }

And the use case to:

 public static void Run()
 {
 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>(cityId);
 break;
 case 2:
 city = CreateCity<HisCity>(cityId);
 break;
 default:
 throw new InvalidOperationException("Invalid Id");
 }
 string json = JsonConvert.SerializeObject(city, Formatting.Indented);
 Console.WriteLine(json);
 Console.ReadLine();
 }
 }
deleted 24 characters in body
Source Link
user73941
user73941

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 byesbuys a building from HisCity can it then change from His to My?)

 City city = null;
 int cityId = -1;
 string strInput = Console.ReadLine();
 int.TryParse(strInput, out cityId);
 if (cityId > 0)
 {
 stringCity jsoncity = string.Empty;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();
 }

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 byes a building from HisCity can it then change from His to My?)

 City city = null;
 int cityId = -1;
 string strInput = Console.ReadLine();
 int.TryParse(strInput, out cityId);
 if (cityId > 0)
 {
 string json = string.Empty;
 switch (cityId)
 {
 case 1:
 city = CreateCity<MyCity, MyBuilding>(cityId);
 break;
 case 2:
 city = CreateCity<HisCity, HisBuilding>(cityId);
 break;
 default:
 throw new InvalidOperationException("Invalid Id");
 }
 json = JsonConvert.SerializeObject(city, Formatting.Indented);
 Console.WriteLine(json);
 Console.ReadLine();
 }

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?)

 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();
 }
Post Undeleted by Community Bot
added 1058 characters in body
Source Link
user73941
user73941

A complete refactoring of youyour 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<TBuilding>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; } }
}
public static IList<TBuilding> GetBuildingData<TBuilding>(int cityId)
where TBuilding : Building, new() {...}

All in all this could simplify your use case to:

 City city = null;
 int cityId = -1;
 string strInput = Console.ReadLine();
 int.TryParse(strInput, out cityId);
 if (cityId > 0)
 {
 string json = string.Empty;
 switch (cityId) {
 case 1:
 city = CreateCity<MyCity, MyBuilding>(cityId);
 break;
 case 2:
 city = CreateCity<HisCity, HisBuilding>(cityId);
 break;
 default:
 throw new InvalidOperationException("Invalid Id");
 }
 json = JsonConvert.SerializeObject(city, Formatting.Indented);
 Console.WriteLine(json);
 Console.ReadLine();
 }

A complete refactoring of you data model could then be:

public abstract class AreaObject
{
 public int Id { get; set; }
 public string Name { get; set; }
 public double Area { get; set; }
}
public class City<TBuilding> : AreaObject where TBuilding: Building
{
 public IList<TBuilding> Buildings { get; set; }
}
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; } }
}
public static IList<TBuilding> GetBuildingData<TBuilding>(int cityId)
where TBuilding : Building, new() {...}

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; } }
}
public static IList<TBuilding> GetBuildingData<TBuilding>(int cityId)
where TBuilding : Building, new() {...}

All in all this could simplify your use case to:

 City city = null;
 int cityId = -1;
 string strInput = Console.ReadLine();
 int.TryParse(strInput, out cityId);
 if (cityId > 0)
 {
 string json = string.Empty;
 switch (cityId) {
 case 1:
 city = CreateCity<MyCity, MyBuilding>(cityId);
 break;
 case 2:
 city = CreateCity<HisCity, HisBuilding>(cityId);
 break;
 default:
 throw new InvalidOperationException("Invalid Id");
 }
 json = JsonConvert.SerializeObject(city, Formatting.Indented);
 Console.WriteLine(json);
 Console.ReadLine();
 }
Post Deleted by Community Bot
deleted 4 characters in body
Source Link
user73941
user73941
Loading
Source Link
user73941
user73941
Loading
lang-cs

AltStyle によって変換されたページ (->オリジナル) /