Better way to retrieve json from an MVC3 Controller Action?
I'm pretty new to MVC3 and I've started work on a newish jQuery / MVC3 / EF 4.1 code first project and have been going over some of the existing code. One part of the code retrieves values from the db via a Controller action when ever the value of a certain drop down changes (SubtypeID.change event). Here's the jQuery:
$.getJSON(
"/StudyDesign/GetSubtypes?typeId=" + typeId,
function (data) {
var theDropDown = document.getElementById("SubtypeID");
if (data.length === 0) {
theDropDown.disabled = true;
}
$.each(data, function () {
$("#SubtypeID").append($('<option>').attr('value', this.SubtypeID).text(this.Name));
});
}
);
And here's the Controller Action:
[OutputCache(Duration = int.MaxValue, VaryByParam = "typeId", Location = OutputCacheLocation.Server)]
public JsonResult GetS开发者_运维百科ubtypes(int typeId)
{
var studyType = this._studyDesignRepository.StudyTypes.SingleOrDefault(s => s.StudyTypeID == typeId);
return studyType == null ? this.Json(new List<Subtype>()) : this.Json(studyType.Subtypes.Select(s => new { s.SubtypeID, s.Name }).ToList(), JsonRequestBehavior.AllowGet);
}
This works fine, but in our Controller constructor we have a lot of code that retrieves different values from the db that populate other dropdowns and grids etc. Once the page is loaded the first time we don't need to to get these values again as they are already in the page, but every time the .change event is fired and StudyDesign/GetSubtypes is called, the Controller constructor runs and does all the db calls again. This seems unnecessary so I'm wondering
- is there a better way to do this?
- can/should we retrieve the json data a different way?
- should we have the json Actions / methods in a separate Controller?
Thanks in advance :)
In general, it's a really bad idea to do a bunch of initialization like that in your controllers constructor. For starters, there's no guarantee that this data will be there from call to call because the stateless nature of the app may cause the controller to get destroyed between calls. This means doing all your expensive work up front all the time.
You should do that expensive work in your Action method, with the cacheing you're using, those methods won't get called again until the cache times out.
Upon re-reading, it sounds like you're talking about other data that gets populated on the page, but never gets changed because you're doing Ajax calls. In that case, my point still stands. Do it in your action method for the page, rather than the constructor. then it will only load that data when you actually retrieve a page.
There are a few different ways of thought when retrieving data. Here are two (one of which I use):
One: Have each action on a controller retrieve only the data they need. They can all share a model, and populate what they will be using in the model. Additionally you can create private methods to populate specific parts of a passed in model so you don't have multiple lines of the same code. private void PopulateColors(Model ModelWithColors)
.
Two (what I use): Have the models populate them selves. For instance, you may have a list of colors as IEnumerable<string>
as a get property, but when the model is passed to a view, or serialized by JSON(model.Colors)
only then is a call to a database made. Initialization values for the models (which might include connection string or whatever else is needed, also could be in the constructor of the model or passed in to a Factory Method).
精彩评论