patterncsharpMinor
Drop-down for populating a list of cities
Viewed 0 times
populatingdropdownforcitieslist
Problem
I have created a drop-down to populate a list of cities. Everything works fine, but I would like to know better ways of doing this. Also, please let me know if it is possible to create the same drop down using `` instead of HTML helpers.
I've used entity framework database first approach to get the data back from database. Please address improvements that need to be done.
I'd also like to know how the lamdba expression works in this scenario:
//ViewModel
public class LocationDTO
{
public IEnumerable Cities { get; set; }
public LocationDTO()
{
this.Cities = new CityDTO[] { };
}
}
public class CityDTO
{
public string CityId { get; set; }
public string CityName { get; set; }
}I've used entity framework database first approach to get the data back from database. Please address improvements that need to be done.
//Controller
Models.LocationDTO Loc = new Models.LocationDTO();
EF.LocationEntities locCtx = new EF.LocationEntities();
public Action Result Index() {
using(locCtx) {
var locResults = (from q in locCtx.usp_GetAllCities()
Select new Models.CityDTO {
CityId = q.Id,
CityName = q.Name });
loc.Cities = locResults.ToList();
}
List citiesList = new List();
Models.CityDTO city = new Models.CityDTO() { CityId = "-1", CityName = "Select City" };
citiesList.Add(city);
citiesList.AddRange(Loc.Cities.ToList());
ViewBag.CitiesDropDown = citiesList;
return view(loc);
}I'd also like to know how the lamdba expression works in this scenario:
//View
@{
List citiesList = ViewBag.CitiesDropDown;
var cityItems = new SelectList(citiesList, "CityId", "CityName");
}
Cities: @Html.DropDownListFor(x => x.Cities.SingleOrDefault().CityID, @cityItems)
Solution
View Model
Looks fine to me, although I'd recommend that if all your DTO classes are to be appended with 'DTO' you instead put them inside a namespace named 'DTO' and leave the 'DTO' characters off the class names entirely.
Controller
Use
Also, I don't like the indentation in your LINQ statement. It makes it difficult to discern between LINQ and constructor.
There is no point in calling
View
You make a call to
Looks fine to me, although I'd recommend that if all your DTO classes are to be appended with 'DTO' you instead put them inside a namespace named 'DTO' and leave the 'DTO' characters off the class names entirely.
Controller
Use
var when the Right Hand Side (RHS) is obvious.Also, I don't like the indentation in your LINQ statement. It makes it difficult to discern between LINQ and constructor.
There is no point in calling
ToList so much. AddRange is going to enumerate the whole collection anyway, so you don't actually need any ToList calls at all.var Loc = new Models.LocationDTO();
var locCtx = new EF.LocationEntities();
public Action Result Index() {
using(locCtx) {
var locResults = (from q in locCtx.usp_GetAllCities()
Select new Models.CityDTO
{
CityId = q.Id,
CityName = q.Name
});
loc.Cities = locResults;
}
var citiesList = new List();
var city = new Models.CityDTO()
{
CityId = "-1",
CityName = "Select City"
};
citiesList.Add(city);
citiesList.AddRange(Loc.Cities);
ViewBag.CitiesDropDown = citiesList;
return view(loc);
}View
You make a call to
Cities.SingleOrDefault(). SingleOrDefault() can return a null value if your City class' default operator returns null (which is the err default behaviour). If this happens, accessing CityId will fail.//View
@{
List citiesList = ViewBag.CitiesDropDown;
var cityItems = new SelectList(citiesList, "CityId", "CityName");
}
Cities: @Html.DropDownListFor(x => x.Cities.SingleOrDefault().CityID, @cityItems)
Code Snippets
var Loc = new Models.LocationDTO();
var locCtx = new EF.LocationEntities();
public Action Result Index() {
using(locCtx) {
var locResults = (from q in locCtx.usp_GetAllCities()
Select new Models.CityDTO
{
CityId = q.Id,
CityName = q.Name
});
loc.Cities = locResults;
}
var citiesList = new List<Models.CityDTO>();
var city = new Models.CityDTO()
{
CityId = "-1",
CityName = "Select City"
};
citiesList.Add(city);
citiesList.AddRange(Loc.Cities);
ViewBag.CitiesDropDown = citiesList;
return view(loc);
}//View
@{
List<TestApp.Models.CityDTO> citiesList = ViewBag.CitiesDropDown;
var cityItems = new SelectList(citiesList, "CityId", "CityName");
}
<div>
Cities: @Html.DropDownListFor(x => x.Cities.SingleOrDefault().CityID, @cityItems)
</div>Context
StackExchange Code Review Q#20586, answer score: 2
Revisions (0)
No revisions yet.