Geeks With Blogs
Doug.Instance Improving the world one post at a time

Recommendation: Don't use the same action names for get and (Ajax) post.

I am always amazed at how bad Microsoft's code examples are.  Code generated using the default templates in Visual Studio is not much better.  To find out, create an empty project (pick your favorite type) and run static code analysis or FxCop and see how many warnings you see.  For some real fun try running StyleCop.  The default templates (and therefore commonly-used standard practices) are also not very good in my opinion.  Consider the following controller code generated using the standard MVC 3 template:

        //
        // GET: /Test/Create

        public ActionResult Create()
        {
            return View();
        } 

        //
        // POST: /Test/Create

        [HttpPost]
        public ActionResult Create(FormCollection collection)
        {
            try
            {
                // TODO: Add insert logic here

                return RedirectToAction("Index");
            }
            catch
            {
                return View();
            }
        }

For those of you who have used MVC for a while, this looks pretty standard.  "So what's the big deal?" you ask. Here is my beef: there is no value to having two controller actions named "Create".  My personal opinion, is that this convention was used to smooth the transition from web forms.  The first Create method would be similar to a Page_Load event handler and the second one would be similar to a Button_Click event handler.  To use more web form language, the first method handles the initial request and the second one handles the "postback".  For people used to dealing with web forms, it makes it more obvious that these two methods are dealing with the same "page".  The only real advantage is that you don't have to specify the action name in Html.BeginForm.  This can be a good thing because it eliminates a magic string from your view.

So let's examine this more closely and see where we start running into problems.  For the sake of argument, let's say that this is a search page.  Therefore you want to use a GET transaction instead of POST so the user can see the URL, bookmark it to re-run their search, etc.  You remove the [HttpPost] attribute. You will now get an error that the action is ambiguous between action methods.

The solution to this problem is simple, just rename one of the methods. My recommendation is to call the first method "New". This then maps to the nice route "/{controller}/New".  In general this is something users expect to see in a URL - what they are looking at so I like to use nouns or adjectives for these actions (Details, New, Info, Order, Product, etc.).  The form would then call the "Create" action.  In this case I like to make my actions that do something use verbs (Create, Update, Delete, List, Rename, etc.).

This convention really starts to make even more sense once you start having multiple Ajax calls from a single page.  After all, why use a paradigm based on full-page posting (Html.BeginForm in MVC) when most applications will use partial-page posts or other script-based methods?

Posted on Thursday, February 16, 2012 11:29 PM MVC | Back to top


Comments on this post: The Enemy of My Friend Writes Bad MVC Controller Actions (Microsoft)

# re: The Enemy of My Friend Writes Bad MVC Controller Actions (Microsoft)
Requesting Gravatar...
Sorry, I don't see the point. The value of having the same names for actions is the possibility to post to the same URL which you previously "get". Simply take care that your model is able to cover both scenarios (get & post). Posting to the same URL is widely used not only by WebForm developers, but also in PHP, Ruby, Python etc. Yes, you can post to another action and then redirect to (or render view of) another action, but thanks to Microsoft that it is not forcing me to do things _only_ this way.
Left by Igor Loginov on Feb 17, 2012 7:17 AM

# re: The Enemy of My Friend Writes Bad MVC Controller Actions (Microsoft)
Requesting Gravatar...
You do realize that once the user performs a post the Url of page will change, right? (Considering there is no redirect. Common case where there are validation errors and the view is rerendered).

Meaning if the user tries to refresh or bookmark the link, he will be hit by a nice error message ...
Left by Pop Catalin on Feb 17, 2012 7:28 AM

# re: The Enemy of My Friend Writes Bad MVC Controller Actions (Microsoft)
Requesting Gravatar...
This feels like you have a hammer and you're trying to turn every problem into a nail. Your second paragraph about the ambiguous match is absolutely correct, but your example is also changing the fundamental concept of what you're trying to accomplish. The way the controller is presented it makes perfect sense to use the same action name. If you change what you're trying to do, that may no longer make sense. Conversely, coming up with a "new standard" to address the second need makes the first scenario more clunky.
Left by Craig Wagner on Feb 17, 2012 11:02 AM

# re: The Enemy of My Friend Writes Bad MVC Controller Actions (Microsoft)
Requesting Gravatar...
Wow! I didn't expect such a fervent response. I have to admit I didn't express myself very well in this post. I added "(Ajax)" to my opening statement so hopefully that will reduce some of the aggression. I will work on an update with more concrete examples to help with my argument.

I definitely will be more careful about making blanket statements in late night posts.
Left by Doug on Feb 17, 2012 7:08 PM

# re: The Enemy of My Friend Writes Bad MVC Controller Actions (Microsoft)
Requesting Gravatar...
Please remember that MVC is also about conventions over configurations. So when the get and post methods share the same name, the framework can figure out what to do and save us some mundane coding.

If I should react to anything in the code example, it would be the catch clause.
Left by Thomas Eyde on Feb 20, 2012 7:29 AM

# re: The Enemy of My Friend Writes Bad MVC Controller Actions (Microsoft)
Requesting Gravatar...
I think you're missing the fact that it's a TEMPLATE, not an example of best practices. It's a basic setup so you dont' have to create a global.asax and add routes everytime you start a new project or have to completely fill in the web.config, etc. Are there mistakes? Well, as Thomas pointed out, YES - that try/catch block merits some smackings. The default HTML in the site.master file, deserves a couple more.
But based on your blog, you know what you're doing, so modify the default routes and change the Actions if you want =)
Left by Carlos on Jun 29, 2012 3:43 PM

Your comment:
 (will show your gravatar)


Copyright © Doug Lampe | Powered by: GeeksWithBlogs.net