は私のコードデザインを評価する - シンプルなスニペット

Question

protected void Page_Load(object sender, EventArgs e) 
{ 
    if (!Page.IsPostBack) 
    { 
     loadCountries(); 
     loadRegions(); 
     loadCities(); 
    } 
} 

private void loadCountries() 
{ 
    Country country = new Country(); 
    ddlCountry.DataSource = country.GetDataTable(); 
    ddlCountry.DataTextField = "countryName"; 
    ddlCountry.DataValueField = "countryID"; 
    ddlCountry.DataBind(); 
} 

private void loadRegions() 
{ 
    Region region = new Region(); 
    ddlRegion.DataSource = region.GetRegionID(ddlCountry.SelectedValue); 
    ddlRegion.DataTextField = "regionName"; 
    ddlRegion.DataValueField = "regionID"; 
    ddlRegion.DataBind(); 

} 

private void loadCities() 
{ 
    City city = new City(); 
    ddlCity.DataSource = city.GetCityID(ddlRegion.SelectedValue); 
    ddlCity.DataTextField = "cityName"; 
    ddlCity.DataValueField = "cityID"; 
    ddlCity.DataBind(); 
} 

protected void ddlCountry_SelectedIndexChanged(object sender, EventArgs e) 
{ 
    loadRegions(); 
    if (ddlRegion.SelectedItem.Text == "No Province") 
    { 
     ddlRegion.Enabled = false; 
     loadCities(); 
    } 
    else 
    { 
     ddlRegion.Enabled = true; 
     loadCities(); 
    } 
} 

コードが

国に関連するビジネスロジックは、国のクラスに入れているのDefault.aspx（プレゼンテーション層）のバックエンドで、同じ規則は、地域や都市に適用されます。

このスニペットのデザインは問題ありませんか？言い換えれば、プレゼンテーションレイヤーの標準デザインを満たしていますか？このスニペットのデザインを改善するにはどうすればいいですか？

私はこれに新しいです、私はそれをゆっくりと確実に取るようにしています。

Answer 1

名前付け規則にはいくつかの作業が必要だと思います。クラスとメソッドの名前について注意深く考えてください。例えば

（クラス名）：

あなたはそれが国を表し示唆国と呼ばれるクラスを持っています。しかし、私はそうは思わない。それは、それが国のデータテーブルを作成する責任があるように見えます。

別の例（メソッド名）：

あなたはRegion.GetRegionIDというメソッドを持っています（）。これはRegionIdに基づいてRegionを取得するように見えますが（私は完全にはわかりません）、GetByRegionIdを好むでしょう。クラス名の批判も同じです。

Answer 2

void loadDropDown(ComboBox cb, Object Source, string textField, string valueField) 
{ 
    cb.DataSource = Source; 
    cb.DataTextField = textField; 
    cb.DataValueField = valueField; 
    cb.DataBind(); 
} 

//loadRegions 
loadDropDown(ddlRegion, new Region().GetRegionID(ddlCountry.SelectedValue), "regionName", "regionID"); 

//etc for the other dropdown lists 

しかし、より完全にあなたの質問に答えるには、それは良いようです。そのコードはすべてUIなので、妥当と思われます。私のコードはDRYのほんの少しの方法でした。

Answer 3

ddlCountry_SelectedIndexChangedではこれはどうですか？

loadRegions（）;

ddlRegion.Enabled =（！ddlRegion.SelectedItem.Text.Equals（ "No Province"））; load301

Answer 4

私は、UIのDataTableを直接使用していることがわかります。長期的には保守性の問題が発生します。

データベースの列名がUIで直接使用されるように、DataTableを使用しています。

したがって、UIコードはDataAccessレイヤーに直接依存しています。

データベースの列名の代わりにビジネスプロパティを使用する必要があると思います。