ASP.NET MVC will produce a controller instance for each request, so you need a bit of help from the framework to assist you with caching - because anything you cache at controller level will only live for a single request's lifetime. As always, Stack Overflow has answers for you :)
// GET: Surveys
// GET: Surveys/Details?name=SomeSurveyName
These comments are somewhat redundant. Unless you've massively changed how routing works in your application, the controller's name and each ActionResult
method already says everything this comment says.
This part could use a more ...vertical layout. Note that when you're using C# object initializer syntax with a parameterless constructor (i.e. new SurveyModel() { ... }
), the parentheses are redundant.
var surveys = surveyMonkey.GetSurveyList()
.Select(s => new SurveyModel
{
Name = s.Nickname,
ResponseCount = s.NumResponses,
Url = s.AnalysisUrl
});
Note that this result is deferring the instantiation of SurveyModel
objects to the view, which iterates the surveys
.
Same here:
var survey = surveyMonkey.GetSurveyList()
.Where(s => s.Nickname == name)
.Select(s => new SurveyModel
{
Name = s.Nickname,
ResponseCount = s.NumResponses,
Url = s.AnalysisUrl
})
.First();
The .First()
is a little bit confusing here. If the monkey API can only ever return 1 survey for a given name, .SingleOrDefault()
would be a much better choice - it would be documenting the fact that a survey has a unique name, and it would return null
when no match is found.
Note that .First()
will throw an InvalidOperationException
- "Sequence contains no element" if the API returns no item for the specified name.