3
\$\begingroup\$

What is the best way to refactor the following code to avoid duplication?

    // JQuery FORM Functions
$(document).on('change','.submittable',function(){
     //save current input field into hidden input 'focus_field' if it exists
        if($('input#focus_field').length){
            var $focus_field = $(this).attr('id')
            $('input#focus_field').val($focus_field)
        }
    $('input[type=submit]').click();
    return false;
});


$(document).on('change','.submittable_wait',function(){
    $('#please_wait').show();
    $('input[type=submit]').click();
    return false;
});


$(document).on('change','.submittable_wait_bar',function(){
    $('#ajax_working').show();
    $('input[type=submit]').click();
    return false;
});

$(document).on('change','.submittable_wait_bar_single',function(){
    $('#ajax_working').show();
    $( this ).closest("form").submit();
    return false;
});

// nested_form - fields added
$(document).on('nested:fieldAdded', function(event) {
    $('#ajax_working').show();
    $('#reload_form').val('true');
    $('input[type=submit].btn-primary').click()
});

// nested_form - fields removed
$(document).on('nested:fieldRemoved', function(event) {
    $('#please_wait').show();
    $('#reload_form').val('true');
    $('input[type=submit].btn-primary').click()
});
\$\endgroup\$
0

1 Answer 1

2
+50
\$\begingroup\$

Some general points:

  • Watch your indentation
  • Watch your whitespace
  • An id is unique on a page - there is no benefit to 'element#id' vs '#id' as a selector
  • It is usually better to store references to elements that don't get removed

Let's apply some of these on the change handler to store the focus field. Here's what we're starting with:

$(document).on('change','.submittable',function(){
    if($('input#focus_field').length){
        var $focus_field = $(this).attr('id')
        $('input#focus_field').val($focus_field)
    }
    $('input[type=submit]').click();
    return false;
});

We don't want to search the DOM for the element every time (unless it can be added/removed) so let's pull it out:

var $focusField = $('#focus_field');

Notice that I've simplified the selector to just the Id. Now we can use our knowledge of jQuery to simplify the next bit of the function.

$(document).on('change', '.submittable', function(e) {
     e.preventDefault();
     $focusField.val(this.id);
     $('input[type=submit]').click();
}

The reason this works is that selecting a set of elements that don't exist in the DOM returns an empty jQuery array which is more than happy to be chained - it just won't do anything. I've also modified your return false to use the recommended preventDefault function on the event object.

If you want you can pull out common functionality into separate functions. For example:

var showAndReload = function (selector) {
    $(selector).show();
    $('#reload_form').val('true');
    $('input[type=submit].btn-primary').click()
};

// nested_form - fields added
$document.on('nested:fieldAdded', function() {
    showAndReload('#ajax_working')
});

// nested_form - fields removed
$document.on('nested:fieldRemoved', function() {
    showAndReload('#please_wait');
});

I'm not really sure it adds much though.

\$\endgroup\$
1

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.