HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Drop-down for populating a list of cities

Submitted by: @import:stackexchange-codereview··
0
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.

//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 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.