MVC selectlist using separation of concerns

This MVC tutorial suggests to make dropdownlists like so:

开发者_开发技巧//
// POST: /StoreManager/Create
[HttpPost]
 public ActionResult Create(Album album)
 {
    if (ModelState.IsValid)
    {
        db.Albums.Add(album);
        db.SaveChanges();
        return RedirectToAction("Index");  
    }
    ViewBag.GenreId = new SelectList(db.Genres, "GenreId",
"Name", album.GenreId);
    ViewBag.ArtistId = new SelectList(db.Artists, "ArtistId",
"Name", album.ArtistId);
    return View(album);
 }

I'm new to MVC, but it seems to me this isn't a good separation of concerns because it makes database calls within the controller. Is this correct?

Is there a better way?

This is up to you to decide on the complexity of your project. The larger the project gets - the least likely you should be to take this approach. I would recommend at the very minimum using the Repository pattern to return your data. Secondly as per the example above it uses ViewBag - this is also something not highly recommended. Sure its 'cool' its there - but generally this data you want in a strongly typed View Model that represents the data going to the View, not the dynamic magic of ViewBag that relies on you not mistyping a field name to use correctly.

I don't think that making database calls is necessarily a bad thing as long as they are querying the database. :)

Of course the usual "it depends" answer applies: size of project, lifetime, complexity, etc.

But for queries a lightweight querying mechanism hits the spot nicely.

You may be interested in looking at Command / Query Responsibility Segregation (CQRS)—but if you do and you are not familiar with it take care not to confuse it with Event Sourcing since in its most basic form CQRS is actually quite a simple idea: separate your reads (queries) from any domain manipulation.

Making database calls that manipulate the state of the data is not all that wise.