HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

jQuery form data

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
formjquerydata

Problem

Could someone please help me make this code as professional as it can be? It works fine, but I feel there's a better way of doing this. I would really appreciate some guidance so I learn to code better.

```
$(function () {
$('#subForm').submit(function (e) {
e.preventDefault();
$.getJSON(
this.action + "?callback=?",
$(this).serialize(),
function (data) {
if (data.Status === 400) {
$('#slidemarginleft p').append("" + data.Message);
$("#slidemarginleft p").append('OK, I'll try again');
$(function() {
var $marginLefty = $('.inner');

$marginLefty.animate({
marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
$marginLefty.outerWidth() : 0
});
});
//add click functionality for the button
$('#slidemarginleft button.wee').click(function() {
var $marginLefty = $('.inner');
$marginLefty.animate({
marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
$marginLefty.outerWidth() : 0
});
setTimeout(function() {
$('#slidemarginleft p').empty();}, 500);
});
} else { // 200
$(function() {
var $marginLefty = $('.inner');
$marginLefty.animate({
marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
$marginLefty.outerWidth() :

Solution

This is typical jQuery mess. If you write a lot of code like this every day, consider switching to frameworks like Backbone or Ember.js which will make your life easier. Concerning this code, here are my remarks:

  • Care more about indentation! All those callbacks are hard enough to read as they are.



  • Only put e.preventDefault(); at the end. If there's a bug in the JS code, the user will be able to use the fallback, that is submitting the form without JS.



  • Don't build html elements with string concatenations, but write things like $('').addClass('css3button') and so on.



-
Move this code into its own function:

var $marginLefty = $('.inner');
$marginLefty.animate({
    marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
        $marginLefty.outerWidth() : 0
});


  • More generally try to separate logic functions from display functions.

Code Snippets

var $marginLefty = $('.inner');
$marginLefty.animate({
    marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
        $marginLefty.outerWidth() : 0
});

Context

StackExchange Code Review Q#22879, answer score: 2

Revisions (0)

No revisions yet.