1

I had an issue which I, after several hours of searching, solved when I stumbled upon the fact that one can set the async-option for an ajax-call to false. My code now works exactly as I expect to, but I wonder if my solution is good or if it can be solved in a better way. If my solution is bad, why do you think so? Async vs sync, what is best? Should one always strive to use async-calls as much as possible?

var pageIndex = 0;

(function () {
   GetData();

   $(window).scroll(function () {
       if ($(window).scrollTop() ==
           $(document).height() - $(window).height()) {
           GetData();
       }
   });
})();

$.ajax({
    type: 'GET',
    url: '/Blog/GetPostsByBlogId',
    data: { "id": @Model.First().ReqBlog.Id, "pageindex": pageIndex },
    dataType: 'json',
    success: function (response) {
         DisplayData(response);
    },
    beforeSend: function () {
         $("#progress").show();
    },
    complete: function () {
         $("#progress").hide();
    },
    error: function (response) {
         alert("Error while retrieving data!");
    }
});

In the success I call the following function:

function DisplayData(response)
{
     if (response != null) {
         $.each(response, function (i, post) {
              $.ajax({
                  async: false,
                  url: "/Blog/GetPostPartialView",
                  data: post,
                  type: "POST",
                  success: function (response) {
                       $("#posts-container").append(response);
                  }
               });
            });
            pageIndex++;
      }
}

Without the "async: false" the data is displayed in a random order for every refresh. With the "async: false" the data displays in the correct order, every time.

Edit:

I used the following solution to avoid using async: false.

My DisplayData-function now looks like this:

function DisplayData(response)
{
     if (response != null) {
         $.each(response, function (i, post) {
             $('#posts-container').append($('<div>').load("/Blog/GetPostPartialView", { Id: post.Id, Title: post.Title, Body: post.Body, Created: post.Created, Updated: post.Updated, Views: post.Views, Blog: post.Blog, Tags: post.Tags, Comments: post.Comments, Author: post.Author }));
         });
         pageIndex++;
     }
}
solojuve1897
  • 165
  • 1
  • 12

2 Answers2

2

async: false is terrible practice. It is a bad solution, and there are very few cases where it would be valid. It's so bad in fact, that if you check the console you'll see a browser warning telling you not to use it.

With regard to your issue of the order being randomised, it's because the AJAX requests in the loop all complete at different times. To solve this you should remove async: false and either:

  1. Collate the returned data together on the client and then sort() it manually before displaying.

  2. Change your server code so that you pass all data in a single AJAX call, then you can return all the required information in the correct order.

The second option is by far the better solution as it also avoids the DDOS-ing of your server that is currently happening due to your (N blog posts + 1) requests that you're currently flooding it with.

Rory McCrossan
  • 331,213
  • 40
  • 305
  • 339
  • Thank you for your reply and your suggested solutions. Why is it bad to use async: false? – solojuve1897 Jul 13 '17 at 07:30
  • 1
    There are several issues, but mainly it's because it blocks the UI thread of the browser from updating the window. This make the browser look to the user like it has crashed and/or hung until the request completes. In cases where the request takes a second or more to complete, that will be very noticeable for your users. [This question](https://stackoverflow.com/questions/6517403/what-are-the-drawbacks-of-using-synchronous-ajax-call) has a more complete explanation. – Rory McCrossan Jul 13 '17 at 07:32
  • Thank you for your help. Much appreciated! :-) – solojuve1897 Jul 13 '17 at 07:33
0

async=false attribute will result in blocking execution of javascript code flow which is generally very bad practice. When you use ajax async=true, you cannot be sure the order of multiple ajax calls. If you cannot do anything in server-side, you can collect the retrived data to a data structure in client-side and order as you wish. You can also call upcoming ajax calls on the completion of previous ajax calls. In that way, you get the results in the order you send the ajax requests.

ali
  • 1,301
  • 10
  • 12
  • "You can also call upcoming ajax calls on the completion of previous ajax calls. In that way, you get the results in the order you send the ajax requests." Do you have any suggestions how I achieve this for my particular case? – solojuve1897 Jul 13 '17 at 07:37
  • For example, you call "Blog/GetPostsByBlogId" for the first Id, in the "success" callback you call for the next Id etc. You can use recursion ajax call such as https://stackoverflow.com/questions/23355168/jquery-recursive-ajax-call-promise. However; this practice will result in many request to your server so you should use it wisely. – ali Jul 13 '17 at 07:44
  • @RoryMcCrossan It is similar but not exactly the same because your are iterating over the returned posts in DisplayData function and send ajax requests to "/Blog/GetPostPartialView" without waiting for the previous one to be completed. You should follow the same approach for this function. – ali Jul 13 '17 at 07:56
  • So you're suggesting that the OP use `async: false`, which the OP is already using now understands is terrible practice and should be avoided... – Rory McCrossan Jul 13 '17 at 07:59
  • It depends on your requirements actually. There might be situations where async=false should be used but generally bad idea. – ali Jul 13 '17 at 08:03
  • Why I at all have an ajax-call inside the "DisplayData" is to render an ASP.NET MVC partialview. I can instead of doing that just write the HTML inside of the append. But I wanted it separated. $("#posts-container").append( // the html here); Using @Html.Partial while sending a model doesn't work inside javascript. So I cant use that. This is all to implement infinite scrolling, loading automatically five new posts when the user scrolls to the bottom. – solojuve1897 Jul 13 '17 at 08:39