javascript looking for any possible flaws and solutions for that

Question

I have this code thats basically reading the dom and sending the values to the server. I am looking for any possible flaws that I may have in this javascript code and any advices to make it better and bug free!

$(document).ready(function () {
    $('.editButton').click(function () {
        var postData = {};
        var cData = {};
        cData.Balance = $(this).parent().children('.balance').text().replace("$", "");
        cData.desiredStatus = $(this).parent().children('.status').text() == "Disbled" ? "enable" : "disable";
        $('#currentBalance').html($(this).parent().children('.balance').text());
        $('#currentStatus').html($(this).parent().children('.status').text());
        $('#desiredStatus').html(cData);

        //set post view
        postData.code = $(this).parent().children('.code').text();
        postData.giftcardaccount_id = $(this).parent().children('.giftcardaccount_id').text();


        $("#dialog").dialog({
            title:"test box",
            modal:true,
            width:700,
            buttons:{
                'Confirm':function () {
                    postData.status = $('#currentStatus').html();
                    var balanceInqury = parseFloat($('#desiredBlance').val());
                    postData.balance = (balanceInqury == 'NaN') ? cData.Balance : balanceInqury;
                    $.ajax({
                        url:'/url/to/postto',
                        type:'POST',
                        data:{
                            postData:postData
                        },
                        success:function (data) {
                            console.log(data);
                            $("#dialog").dialog("close");

                        },
                        error:function () {
                        }
                    });
                },
                Cancel:function () {
                    $(this).dialog("close");
                }
            }
        });//dialog
    });


    $('#changeStatus').click(function () {
        var c = $('#currentStatus').html() == 'Disabled' ? 'Enabled' : 'Disabled';
        var d = $('#desiredStatus').html() == 'disable' ? 'enable' : 'disable';
        $('#currentStatus').html(c);
        $('#desiredStatus').html(d);

    });

});

Answer 1

• You're creating a lot of jQuery objects in there. It would be much more efficient to just create them once. For example:

  var postData = {},
      cData = {},
      $this = $(this),
      $parent = $this.parent(),
      $balance = $parent.children(".balance");
  

As a general rule, I try to avoid calling any method on the same object more than once. For any methods you need to call repeatedly, just call it once and store the result.

• The following line seems to be incorrect. You're passing an object to the .html() method (which it doesn't accept):

  $('#desiredStatus').html(cData);
  
  //Did you mean to do this instead?
  $('#desiredStatus').html(cData.desiredStatus);
  

• You are creating a new modal dialog every time a .editButton element is clicked. You could move the creation of the dialog outside of the event handler and just call $("#dialog").dialog("open") when you want it to appear.

• The comparison balanceInqury == 'NaN' will never be true. JavaScript is funny like that, NaN !== NaN. There is a built-in isNaN function you can use though:

  postData.balance = isNaN(balanceInqury) ? cData.Balance : balanceInqury;
  

• For shorter code you could replace your call to $.ajax with a call to the shorthand $.post method. If you'd rather not do that, you can at least remove the error property from the options object, since it doesn't do anything in your case:

  $.post('/url/to/postto', { postData: postData}, function (data) {
      //You make another unnecessary jQuery object in here. Cache the #dialog object!
  });
  

• Finally, you could change the code in the #changeStatus click event handler by passing a function to the .html method, which in my opinion is a bit neater:

  $('#currentStatus').html(function () {
      return $(this).html() === "Disabled" ? "Enabled" : "Disabled"
  });
  $('#desiredStatus').html(function () {
      return $(this).html() === "disable" ? "enable" : "disable";
  });