1
\$\begingroup\$

What's a better and more elegant way of writing this:

$('#footer form').submit(function(event) {
    event.preventDefault(event);
    $('#footer form .preload').spin(opts);
    $.post('contact.php', $(this).serialize(), function(response) {
        $('#footer form .preload div').remove();
        $('#footer form h5').remove();
        $('#footer form .error').html('');
        if (response == 1) {
            $('#footer form h3').after('<h5>Your message has been sent!</h5>');
            $('#footer form input[type=submit]').fadeOut();
        } else if (response == 0) {
            $('#footer form h3').after('<h5>A problem occured. Please try again later.</h5>');
            $('#footer form input[type=submit]').fadeOut();
        } else {
            var errors = $.parseJSON(response);
            $('label[for=name] .error').html(errors.name);
            $('label[for=email] .error').html(errors.email);
            $('label[for=message] .error').html(errors.message);
        }
    });
});
\$\endgroup\$
2
\$\begingroup\$

First thing I would do is cache some selectors, like the form:

$form = $('#footer form');

And then you can use it like:

$form.find('h3');
\$\endgroup\$
  • 1
    \$\begingroup\$ To clarify the reason for this: each call to e.g. $('#footer form') has an overhead. \$\endgroup\$ – yitznewton Feb 15 '12 at 15:59
  • \$\begingroup\$ What do you mean by overhead? \$\endgroup\$ – simon Feb 16 '12 at 1:56
0
\$\begingroup\$

This answer is really just an experiment. I wanted to see if I could make it read better.

I found that I had to process each line and ask "what does this do?". I wanted to eliminate that with named functions.

I do like a few patterns of this solution:

  1. separation of the messages
  2. how the messages are found; and how easy it would be to add another
  3. displaying the loader and cleaning up element with the ajax call
  4. logging an error is separate and could be detached completely

Here's the code:

$('#footer form').submit(function(event) {
  event.preventDefault(event);

var $this = $(this),
    messages = {
        '0': 'A problem occured. Please try again later.',
        '1': 'Your message has been sent!'
    }

$.ajax({
    url: 'contact.php',
    data: $this.serialize(),
    type: 'POST',
    success: displayResponse,
    beforeSend: prepareForResponse,
    complete: hideLoader
});

function displayResponse(response) {
    key = response.toString();

    if (isMessage()) {
        displayMessage(messages[key]);
        $this.find('input[type=submit]').fadeOut();
    } else {
    logError(response);
    }

  function isMessage() {
    return key in messages
      }
    };

    function displayMessage(message) {
        var $h5 = $("<h5 />").text(message);
        $this.find('h3').after($h5);
    }

  function logError(error) {
    var errors = $.parseJSON(error);

    $('label[for=name] .error').html(errors.name);
    $('label[for=email] .error').html(errors.email);
    $('label[for=message] .error').html(errors.message);
  }

    function prepareForResponse() {
        showLoader();
        clearResponses();
    }

  function showLoader() {
    $this.find('.preload').spin(opts);  
  }

    function clearResponses() {
    $this.find('form h5').remove();
    $this.find('.error').html('');
    }

    function hideLoader() {
        $this.find('.preload div').remove();
    }
});

(nothing is tested)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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