Use viewbags for metadata or place them in a ViewModel

3

In the scenario below, I have a View that depends on some ViewBags to fill options on selects html . When performing POST and in case something goes wrong in the register, the user is redirected to the same View with the filled fields. In this method POST , I am currently replicating the creation of ViewBags . Is this the best approach? Is it correct to put the list of metadata (States, cities, neighborhoods, countries) in ViewModel and load selects by it?

public ActionResult Cadastrar()
{
       ViewBag.Estados =[...]; //List de selectListItem
       ViewBag.Cidades = [...];
       ViewBag.Bairros = [...];
       ViewBag.Paises = [...];

       return View();
}

[HttpPost
public ActionResult Cadastrar(CadastrarUsuarioViewModel model)
{
       ViewBag.Estados =[...]; //List de selectListItem
       ViewBag.Cidades = [...];
       ViewBag.Bairros = [...];
       ViewBag.Paises = [...];
       try
       {
           //DoSomething
           ExibirMensagemSucesso();
           return RedirectToAction("Index");
       }catch (Exception ex)
       {
            //Do Something
            return View(model);
       }       
}
    
asked by anonymous 06.05.2015 / 13:18

1 answer

2

This order of instructions here is not good:

[HttpPost]
public ActionResult Cadastrar(CadastrarUsuarioViewModel model)
{
       ViewBag.Estados =[...]; //List de selectListItem
       ViewBag.Cidades = [...];
       ViewBag.Bairros = [...];
       ViewBag.Paises = [...];
       try
       {
           //DoSomething
           ExibirMensagemSucesso();
           return RedirectToAction("Index");
       }catch (Exception ex)
       {
            //Do Something
            return View(model);
       }       
}

First, catching exceptions must be done in the OnException " of Controller , not Action .

Second, that the item load must be after the primary business rule of an Action decorated with [HttpPost] , not before. This is simply because if the rule works, you will not return the same again, so you do not have to load the ViewBags again.

Third, you do not validate the ViewModel . This is done by ModelState . through the property IsValid . There is a high chance that the application executes unwanted logic there.

Change to the following:

[HttpPost]
public ActionResult Cadastrar(CadastrarUsuarioViewModel model)
{
       if (ModelState.IsValid) 
       {
           //DoSomething
           ExibirMensagemSucesso();
           return RedirectToAction("Index");
       }

       ViewBag.Estados =[...]; //List de selectListItem
       ViewBag.Cidades = [...];
       ViewBag.Bairros = [...];
       ViewBag.Paises = [...];
       return View();
}

No more, it's ok.

    
21.08.2015 / 20:43