1
\$\begingroup\$

As you can see I have to populate the ViewModel with the List of Tags so that the dropdown can have all the options for the user to select. And I have to do this 2 times for the Create Action and 2 times for the Edit action.

[Authorize]
public ActionResult Create()
{
    var tags = from i in _articleService.GetAllTags()
               select new SelectListItem {Text = i.Name, Value = i.ID.ToString()};
    var model = new CreateArticleViewModel {Tags = tags};
    return View(model);
}

[HttpPost, Authorize]
public ActionResult Create(CreateArticleViewModel model)
{
    if (ModelState.IsValid)
    {
        try
        {
            var currentUser = _userService.GetByUserName(User.Identity.Name);
            var tagIds = model.TagIds.Select(int.Parse);
            var article = _articleService.Create(currentUser, model.Title, model.Body, tagIds);
            return RedirectToAction("ViewArticle", new { id = article.ID });
        }
        catch (Exception ex)
        {
            ModelState.AddModelError("Error", ex.Message);
        }
    }
    var tags = from i in _articleService.GetAllTags()
               select new SelectListItem { Text = i.Name, Value = i.ID.ToString() };
    model.Tags = tags;
    return View(model);
}

[Authorize]
public ActionResult Edit(int id)
{

    var article = _articleService.GetById(id);
    var tags = from tag in _articleService.GetAllTags().ToList()
               select new SelectListItem { Text = tag.Name, Value = tag.ID.ToString(), Selected = article.HasTag(tag)};
    var model = new CreateArticleViewModel {Title=article.Title, Body = article.Body, Tags = tags };


    return View(model);
}

[HttpPost, Authorize]
public ActionResult Edit(CreateArticleViewModel model)
{
    if (ModelState.IsValid)
    {
        try
        {
            //handle article update
        }
        catch (Exception ex)
        {
            //Add ModelState error 
        }
    }
    var tags = from tag in _articleService.GetAllTags().ToList()
               select new SelectListItem { Text = tag.Name, Value = tag.ID.ToString(), Selected = article.HasTag(tag)};
    model.Tags= tags;

    return View(model);
}

Is there a better way?

\$\endgroup\$
1
\$\begingroup\$

You could refactor out your tagging into a helper method, as below:

[Authorize]
public ActionResult Create()
{
   var model = new CreateArticleViewModel {};
   TagModel (model, _ => false);
   return View(model);
}
[HttpPost, Authorize]
public ActionResult Create(CreateArticleViewModel model)
{
   if (ModelState.IsValid)
   {
      try
      {
         var currentUser = _userService.GetByUserName(User.Identity.Name);
         var tagIds = model.TagIds.Select(int.Parse);
         var article = _articleService.Create(currentUser, model.Title, model.Body, tagIds);
         return RedirectToAction("ViewArticle", new { id = article.ID });
      }
      catch (Exception ex)
      {
         ModelState.AddModelError("Error", ex.Message);
      }
   }
   TagModel (model, _ => false);
   return View(model);
}

[Authorize]
public ActionResult Edit(int id)
{
   var article = _articleService.GetById(id);
   var model = new CreateArticleViewModel {Title=article.Title, Body = article.Body};
   TagModel (model, tag => article.HasTag (tag));

   return View(model);
}

[HttpPost, Authorize]
public ActionResult Edit(CreateArticleViewModel model)
{
   if (ModelState.IsValid)
   {
      try
      {
            //handle article update
      }
      catch (Exception ex)
      {
            //Add ModelState error 
      }
   }
   TagModel (model, tag => article.HasTag (tag));
   return View(model);
}

private void TagModel (CreateArgicleViewModel model, Predicate<Tag> isSelected)
{
   var tags = from tag in _articleService.GetAllTags()
              select new SelectListItem { Text = tag.Name, Value = tag.ID.ToString(), Selected = isSelected(tag)};
   model.Tags= tags;
}
\$\endgroup\$
1
\$\begingroup\$

Recently I've been reading a great series of blog posts by Ayende Rahien discussing how we can limit abstractions in a code base and how to implement a very simple infrastructure to make an abstraction for code reuse. Take a look at one of the posts of these series here. You can find the rest of them on his blog.

\$\endgroup\$
  • \$\begingroup\$ great series of blogposts... I dont see how it relates to my question though. \$\endgroup\$ – nacho10f Feb 27 '12 at 20:22
  • \$\begingroup\$ You can use Query abstraction to run a predefined query to get all the tags \$\endgroup\$ – Beatles1692 Feb 27 '12 at 21:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.