views:

94

answers:

3

I'm reviewing some code for a colleague and while there's nothing inherently wrong with the jQuery Ajax calls I'm looking at, I would like to be more certain about what should and should not appear in a normal Ajax call to an ASP.Net MVC controller action.

For example, in the following code:

    $(function() {
        $.ajax({
            url: "/Controller/action",
            type: "POST",
            data: ({ myObjectOrParameters }),
            success: function(data) { alert(data); }
        });
    });

Is this pattern just fine as-is, or are there other things that should also be there? Is contentType advisable? What about dataFilter? Is that unnecessary since we aren't using Microsoft Ajax and aren't concerned about the ".d" that it returns, should I even worry?

What about the type? Is it best practice to use "GET" or even "PUT" when reading or updating information, or is "POST" the most appropriate to use in every case?

Is it more appropriate to use $.ajaxSetup in every case, or can we get away with explicitly defining our arguments each time?

+3  A: 

Requests made with GET should be idempotent. (Meaning that if they are repeated, the net effect is the same). A simple subset of idempotent queries are those with no side effects. e.g. a search query. A good rule of thumb is that things that update the user's state should be POST. Google for more info on GET vs POST.

Another best practice that you're missing is an error handler. This handles things like a 500 or 403 server error in response to the request, and is essential.

Kyle Butt
+1 For handling errors.
Jonathan Sampson
+5  A: 

Call me a man of brevity...

I would prefer seeing the $.post() method used in this case. Unless you're using the more esoteric options in $.ajax(), I don't see any reason to use it when there are shorter and more succinct methods available:

$.post("/Controller/action", { myObjectOrParameters }, function(data) {
  alert(data);
});
Jonathan Sampson
An error handler is not esoteric.
Kyle Butt
@KyleButt: 'esoteric' in the sense that it's not commonly included in most uses. I used that term tongue-in-cheek.
Jonathan Sampson
That's actually a very good point. It's easy to overlook all the (many) additional Ajax functions.
Phil.Wheeler
+1  A: 

As mentioned in previous posts the type of request your making is dependent on the type of action to take.

  • GET: the state of the system should not be changed (as Kyle wrote, idempotent).
  • POST: sending data which will update a value or state of the system.

The are other type's of requests HEAD, DELETE, etc. But, those are not commonly used outside of RESTful development (http://en.wikipedia.org/wiki/Representational_State_Transfer).

A practice I have been using when developing a website which relies on javascript/ajax is to develop a custom javascript framework for the website on top of jQuery. The library will handle common functionality that is specific to the website. For example you question is about jQuery's ajax function. Some common functionality that is specific to your website might be: displaying error messages, handling unexpected error codes (500, 404, etc), adding common parameters to calls, and the data transfer type (JSON, XML, etc).

A simple custom javascript framework for a website could look like this:

(function ($) {
    if (!window.myWebsite) { window.myWebsite = {}; }

    //  once myWebsite is defined in the "window" scope, you don't have
    //  to use window to call it again.
    $.extend(myWebsite, {
        get: function (url, data, callback) {
            myWebsite._ajax(url, data, "GET", callback);
        },

        post: function (url, data, callback) {
            myWebsite._ajax(url, data, "POST", callback);
        },

        _ajax: function (url, data, type, callback) {
            //  http://api.jquery.com/jQuery.ajax/
            $.ajax({
                type: type,
                url: url,
                data: data,
                dataType: 'json',
                success: function(data, status, request) {
                    //  I'll talk about this later. But, I'm assuming that the data
                    //  object returned from the server will include these fields.
                    if( data.result == 'error' ) {
                        myWebsite._displayError( data.message );
                    }

                    //  if no error occured then the normal callback can be called
                    if( $.isFunction(callback) )
                        callback();
                },
                error: function (request, status, error) {
                    myWebsite._displayError( error );        

                    //  you can also use this common code for handling different
                    //  error response types. For example, you can handle a
                    //  500 "internal server error" differently from a 404
                    //  "page not found"
                }
            });
        },

        _displayError: function( text ) {
            //  Many pages have a common area
            //  defined to display error text, let's call that
            //  area <div id="errorDiv" /> on your website
            $('#errorDiv').text(error);
        }
    });
})(jQuery);

You can call your custom javascript from the page like so:

myWebsite.get( '/Controller/Action', {}, function() { ... } );

Once you have the base javascript framework in place, then you can add classes to your ASP.NET MVC project which will return data which the framework will expect. In the above javascript, the _ajax function has a success function which expects a JSON object that contains the properties 'result' and 'message'. This can be implemented through a base class within your MVC model.

using System;

/// <summary>
/// <para>
/// Encapsulates the common/expected properties for the JSON results
/// on this website.
/// </para>
/// <para>
/// The <see cref="result" /> property should contain the value 'success', when
/// all processing has gone well. If the action has either 'fail'ed or has an 'error'
/// then the <see cref="message"/> property should also be filled in.
/// </para>
/// </summary>
public abstract class JsonResultBase
{

    #region constructors

    /// <summary>
    /// Creates a basic <see cref="JsonResultBase"/> with a 'success' message.
    /// </summary>
    public JsonResultBase()
        : this("success", string.Empty) { }

    /// <summary>
    /// Creates a <see cref="JsonResultBase"/> with the <see cref="result"/> and <see cref="message"/>
    /// properties initialized. This should be used when creating a 'fail'
    /// result.
    /// </summary>
    /// <param name="result">The result type: 'sucess', 'fail', or 'error'.</param>
    /// <param name="message">The message which described why the result occured.</param>
    public JsonResultBase(string result, string message)
    {
        if (result != "success" && string.IsNullOrEmpty(message))
        {
            throw new ArgumentException("message", "message must have a value when the result is not 'success'.");
        }

        this.result = result;
        this.message = message;
    }

    /// <summary>
    /// Creats a <see cref="JsonResultBase"/> which translates an exception into
    /// an error message for display on the webpage.
    /// </summary>
    /// <param name="e">The exception to send back.</param>
    public JsonResultBase(Exception e)
    {
        this.result = "error";
        this.message = e.Message;
    }

    #endregion

    #region properties

    /// <summary>
    /// The result of the action. This could contain the value of 'success', 'fail', or 'error'.
    /// Or, some other values that you define.
    /// </summary>
    public string result { get; set; }

    /// <summary>
    /// Any extra information which would be helpful to describe a result. This will always be
    /// populated if the result is not 'success'.
    /// </summary>
    public string message { get; set; }

    #endregion

}

That base class can then be extended to return specific data for a call and used within the controllers.

public class HomeController : Controller
{

    private class ValuesJsonResult : JsonResultBase {
        public ValuesJsonResult() : base() {}
        public ValuesJsonResult(Exception e) : base(e) {}

        public string[] values  = new string[0];
    }

    public ActionResult GetList() {
        try {
            return Json(
                new ValuesJsonResult{ values = new [] { "Sao Paulo", "Toronto", "New York" } },
                JsonRequestBehavior.AllowGet
            );
        } catch( Exception e ) {
            //  Opps, something went wrong
            return Json( new ValuesJsonResult(e), JsonRequestBehavior.AllowGet );
        }
    }

}

HTH

smaglio81