Dealing with repeating code in asp.net mvc 3 controller

Question

I'm working on a controller that deals with forms/documents and the more I advance with the tasks the more repeating parts of code I see in my methods. It's my first ASP application at all mvc or not and I'm not sure what is the best way to optimize my code. Something that I've noticed - a pattern that is repeated several time is this :

public ActionResult DisplayForm(int? documentId, long status)
        {
            ViewBag.Status = status;

            List<MCS_DocumentFields> model = (List<MCS_DocumentFields>)DocumentFieldService.GetFieldsForDocument(documentId);

            var finalModel = model
                .OrderBy(c => c.ContentTypeId)
                .ThenBy(c => c.RowNo)
                .ThenBy(c => c.ColumnNo)
                .ThenBy(c => c.MCS_Fields.Order)
                .ToList();
            return View(finalModel);
        }

This is a method that displays a certain form. But when the form is edited I deal with this in another method :

 public ActionResult UpdateDocument(List<MCS_DocumentFields> collection)
        {
            //TODO deal with the repeating code
            int? documentId = (int)collection[0].MCS_Documents.Id;
            ViewBag.Status = 1;
            List<MCS_DocumentFields> model = (List<MCS_DocumentFields>)DocumentFieldService.GetFieldsForDocument(documentId);

            var finalModel = model
                .OrderBy(c => c.ContentTypeId)
                .ThenBy(c => c.RowNo)
                .ThenBy(c => c.ColumnNo)
                .ThenBy(c => c.MCS_Fields.Order)
                .ToList();

            //var ts = collection;
            return View("DisplayForm", finalModel);

        }

I have to implement the logic for data validation and updating, but at the end I want to show the same view - the edited form with the new data and some proper message like "Save successfull" or something like this.

So I wonder what can I do here - write some private methods in my controller which I'll call where needed. Maybe there's a way to... as in the example - deal with the saving in UpdateDocument method, but then UpdateDocument to return DisplayForm method.. I'm not sure.

TheKingDave · Accepted Answer · 2013-05-27 11:29:54Z

Without knowing more about your application I suggest doing the sorting within the DocumentFieldService.GetFieldsForDocument method if you can.

If you cannot do that you could do something like this:

public List<MCS_DocumentFields> GetSortedFieldsForDocument(documentID)
        {

            List<MCS_DocumentFields> model = (List<MCS_DocumentFields>)DocumentFieldService.GetFieldsForDocument(documentId);

            var sorted = model
                .OrderBy(c => c.ContentTypeId)
                .ThenBy(c => c.RowNo)
                .ThenBy(c => c.ColumnNo)
                .ThenBy(c => c.MCS_Fields.Order)
                .ToList();

            return sorted;
        }

As a private method like you say then you can do:

public ActionResult DisplayForm(int? documentId, long status)
        {
            ViewBag.Status = status;
            return View(GetSortedFieldsForDocument(documentId));
        }

and

public ActionResult UpdateDocument(List<MCS_DocumentFields> collection)
        {
            //TODO deal with the repeating code
            int? documentId = (int)collection[0].MCS_Documents.Id;
            ViewBag.Status = 1;

            return View("DisplayForm", GetSortedFieldsForDocument(documentId));

        }

In your controller.

Arjan Einbu · Accepted Answer · 2013-05-27 11:42:00Z

1

You can return with a call to your DisplayForm method, something like this:

public ActionResult UpdateDocument(List<MCS_DocumentFields> collection)
{
    //TODO deal with the repeating code
    int? documentId = (int)collection[0].MCS_Documents.Id;
    long status = 1;
    List<MCS_DocumentFields> model = (List<MCS_DocumentFields>)DocumentFieldService.GetFieldsForDocument(documentId);

    return DisplayForm(documentId, status);
 }

And in DisplayForm, you should specify the name of the view to render, changing

return View(finalModel);

with

return View("DisplayForm", finalModel);

Addendum

You should take a look at the Post-Redirect-Get or PRG pattern: http://en.wikipedia.org/wiki/Post/Redirect/Get

When following that pattern, you could instead in the end of the UpdateDocument method, return a redirect to DisplayForm.

return RedirectToAction("DisplayForm", new{ documentId, status });

answered May 27, 2013 at 11:42

Arjan Einbu

13.7k2 gold badges59 silver badges59 bronze badges

1 Comment

Leron Over a year ago

Thnaks, it doesn't seem like many people want to tell something on this topic so I'll accept your answer and gonna try your suggestions to optimize my code.

Collectives™ on Stack Overflow

Dealing with repeating code in asp.net mvc 3 controller

2 Answers 2

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related