Оцените мой дизайн кода - простой фрагмент

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 (уровень представления)

Любая бизнес-логика, связанная со страной, помещается в класс Country, те же правила применяются к региону и городу.

Этот фрагмент кода в порядке?Другими словами, соответствует ли он стандартному дизайну уровня представления?Как я могу улучшить этот фрагмент кода (если это возможно)?

Я новичок в этом, я стараюсь делать это медленно, но верно.

Answer 1

Я думаю, что соглашения по именованию требуют некоторой работы.Тщательно продумайте названия ваших классов и методов.

Например (имя класса):

У вас есть класс с именем Country, который предполагает, что он представляет страну.Но я не думаю, что это так.Похоже, он отвечает за создание таблицы данных о странах.

и другой пример (имя метода):

У вас есть метод с именем Region.GetRegionID ().Это выглядит (но я не совсем уверен), как будто это получает Region на основе RegionId, поэтому я бы предпочел 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

Но чтобы более полно ответить на ваш вопрос, это выглядит хорошо. Этот код весь пользовательский интерфейс, так что это кажется разумным. Мой код был просто маленьким способом СУХОЙ.

Answer 3

Я вижу, что вы непосредственно используете DataTable в пользовательском интерфейсе.Это приведет к проблемам с поддержкой в ​​долгосрочной перспективе.

Вы используете DataTable, поэтому имя столбца базы данных используется непосредственно в пользовательском интерфейсе.

Таким образом, ваш код пользовательского интерфейса напрямую зависит от DataAccesslayer.

Я думаю, что вы должны использовать Business Properties вместо имен столбцов базы данных.

Answer 4

Как насчет этого в ddlCountry_SelectedIndexChanged?

loadRegions ();

ddlRegion.Enabled = (! DdlRegion.SelectedItem.Text.Equals ("Нет провинции"));

loadCities ();