Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

My code is all working fine. But I have a couple of questions about how I am structuring everything.

  1. Is having all of my scripts in one "scripts.js" file like this good practice?
  2. Is my structure organised and logical or is there another best practice or way it could be substantially improved?
  3. Am I doing anything that will be significantly reducing the performance of my scripts and is there a way to rectify this?

Pasting in my entire file. It's a fair amount. But as I'm only after a general structural appraisal it should be okay.

jQuery(document).ready(function($) {


    var offCanvas = $(".off-canvas");
    var bookForm = false;
    var moving = false;
    var cardsMoving = false;
    var card = $(".card");
    var cardCounter = 0;
    var cardWrap = $(".card-wrap");
    var homeasidediv = $(".home-aside");
    var canvasWrap = $(".canvas-wrapper");
    var arb = $(".arb");
    var slideWrap = $(".slider-wrap");
    var navLink = $(".top-bar ul a");
    var href;
    var block = $("div#block");
    var validator = new FormValidator('booking-form', [
        {
            name: 'name',
            display: 'name',    
            rules: 'required'
        }, {
            name: 'email',
            display : 'email',
            rules: 'required|valid_email'
        }, {
            name: 'quantity',
            display: 'number of guests',    
            rules: 'required'
        }
        ], function(errors, evt) {
            if (errors.length > 0) {
                $(".booking-errors").fadeIn();
                for (var i = 0, errorLength = errors.length; i < errorLength; i++) {
                        $(".booking-errors ul").append('<li>' + errors[i].message + '</li>');
                }

            }
    });
    var app = {
        init : function(){
        /*-----------------------------------
        INITIALISATION
        ------------------------------------*/
            canvasLoader();
            makeMaxHeight($(".canvas-wrapper"));
            textFit();
            slideSize(slideWrap, 1.25);
            homeAside();
            hexSize();
            fbInit();
            mapInit($('#footer-map'));
            mapInit($('#venue-map'));
        }, 
        initdelayed : function(){
        /*-----------------------------------
        INITIALISATION (delayed)
        ------------------------------------*/
            makeMaxHeight(cardWrap);
            verticalAlign($(".card-wrap"), $(".card-wrap img"));
            fbPosts();
            doneLoading();
        },
        /*-----------------------------------
        SCREEN RESIZE
        ------------------------------------*/
        resize : function(){
            makeMaxHeight($(".canvas-wrapper"));
            makeMaxHeight(cardWrap);
            slideSize(slideWrap, 1);
            homeAside();
            hexSize();
            verticalAlign($(".card-wrap"), $(".card-wrap img"));
        }
    }
    //call init
    app.init();
    //call init delayed 
    setTimeout(function(){
        app.initdelayed();
    },250);
    //call resize
    $(window).resize(function(){
        app.resize();
    });
    /*-----------------------------------
    EVENTS
    ------------------------------------*/

    //Update Paypal Return URL

    $("#booking-form").on("input paste", function(){
        custDetails();
    });
    //cancel booking form by clicking anywhere
    $("body").on("click",function(e){
        if (bookForm === true && moving === false && $(e.target).closest(".off-canvas").length === 0) {
            moveCanvas("off");
        };
    });
    //toggle booking form by clicking button
    $(".book").on("click", function(e){
        if (bookForm === false) {
            moveCanvas("on");
        } else {
            moveCanvas("off");
        }

    });
    //show booking form by swiping left
    Hammer("body").on("swipeleft", function() {
        if (bookForm === false) {
            moveCanvas("on");
        };
    });
    //cancel booking form by swiping right
    $("body").on("swiperight", function() {
        if (bookForm === true) {
            moveCanvas("off");
        };
    });
    // Keep sticky menu shrunk when off canvas menu is visible
    $(window).scroll(function(){
        if (bookForm === true) {
            $(".navbar.sticky.fixed").css("left", -offCanvas.width());
        };
    });
    // move "cards" down when user touches or clicks them
    card.on("touch", function() {
        cardsDown();
    });
    // move "cards" down when user scrolls down with cursor over them
    $(window).on('mousewheel', function(event) {
        if (event.originalEvent.wheelDelta <= 0) {
            if ($(".card-wrap:hover").length) {
                cardsDown();
            }
        } else {
            if ($(".su:hover").length) {
                cardsUp();
            }
        }
    });
    // fade in hexagons when they come into view
    $(".hex-grid").waypoint(function() {
         $(".hex").each(function(i,e){
            setTimeout(function(){
                $(e).removeClass("transparent");
            }, 150*i);
         });
    }, { offset: "60%" });
    // Scroll down when user clicks "Learn More" button
    $(".bgcard img").on("click", function(){
        $("body").animate({
            scrollTop : $(".slider-wrap").offset().top - $("body").offset().top + $("body").scrollTop() - 55
        }, 1000);​
    });
    // Slide-in transitions when element comes into view
    setTimeout(function(){
        $(".sliding").waypoint(function() {
             $(this).addClass("in");
        }, { offset: "80%" });
    }, 100);
    //fade in secondary logos
    $(".silver-logos").waypoint(function() {
         $("img").each(function(i,e){
            setTimeout(function(){
                $(e).removeClass("transparent");
            }, 150*i);
         });
    }, { offset: "80%" });
    //get directions
    $("#directions").on("click", function(){
        getDirected($("#postcode").val());
    });

    /*-----------------------------------
    FUNCTIONS
    ------------------------------------*/

    function verticalAlign(parent, img){
        img.css("margin-top", (parent.height() - img.height())/2);
    }
    function textFit(){
        $("h2.fit-text").fitText(1);
        $("p.fit-text").fitText(1.3);
    }
    function makeMaxHeight(obj) {
        obj.height($(window).innerHeight());
    }
    function moveCanvas(onoff){
        if (onoff === "on") {
            moving = true;
            $(".canvas-wrapper").width(offCanvas.width());
            $(".navbar.sticky").css("left", -offCanvas.width());
            arb.css("left", -offCanvas.width());
            bookForm = true;
            setTimeout(function(){
                moving = false;
            },600);
        } else {
            $(".canvas-wrapper").width(1);
            $(".navbar.sticky").css("left", "0");
            arb.css("left", "0");
            bookForm = false;
        }
    }
    function cardsDown(){
        if (cardCounter <=3 && cardsMoving === false) {
            $("body").addClass("hacky");
            cardsMoving = true;
            $(".card").eq(cardCounter).addClass("dropped");
            cardCounter++;
            setTimeout(function(){
                $("body").removeClass("hacky");
                cardsMoving = false;
            }, 600);
        };
    }
    function cardsUp(){
        if (cardCounter >=0 && cardsMoving === false) {
            $("body").addClass("hacky");
            cardsMoving = true;
            $(".card").eq(cardCounter).removeClass("dropped");
            cardCounter--;
            setTimeout(function(){
                $("body").removeClass("hacky");
                cardsMoving = false;
            }, 600);
        };
    }
    function slideSize(obj, radix, bool){
        obj.innerHeight($("img.main-slider").height()/radix);
    }
    function homeAside() {
        if ($(".home-aside").outerHeight() >= $(".slider-wrap").height()/1.2) {
            homeasidediv.addClass("too-big");
        } else {
            homeasidediv.removeClass("too-big");
        };
    }
    function hexSize(){
        $(".hex").height($(".hex").width()/1.725);
    }
    twitterFetcher=function(){function v(e){return e.replace(/<b[^>]*>(.*?)<\/b>/gi,function(c,e){return e}).replace(/class=".*?"|data-query-source=".*?"|dir=".*?"|rel=".*?"/gi,"")}function m(e,c){for(var g=[],f=RegExp("(^| )"+c+"( |$)"),a=e.getElementsByTagName("*"),d=0,b=a.length;d<b;d++)f.test(a[d].className)&&g.push(a[d]);return g}var w="",j=20,q=!0,h=[],r=!1,n=!0,p=!0,s=null,t=!0,x=!0,u=null;return{fetch:function(e,c,g,f,a,d,b,k,l){void 0===g&&(g=20);void 0===f&&(q=!0);void 0===a&&(a=!0);void 0===
    d&&(d=!0);void 0===b&&(b="default");void 0===k&&(k=!0);void 0===l&&(l=null);r?h.push({id:e,domId:c,maxTweets:g,enableLinks:f,showUser:a,showTime:d,dateFunction:b,showRt:k,customCallback:l}):(r=!0,w=c,j=g,q=f,p=a,n=d,x=k,s=b,u=l,c=document.createElement("script"),c.type="text/javascript",c.src="//cdn.syndication.twimg.com/widgets/timelines/"+e+"?&lang=en&callback=twitterFetcher.callback&suppress_response_codes=true&rnd="+Math.random(),document.getElementsByTagName("head")[0].appendChild(c))},callback:function(e){var c=
    document.createElement("div");c.innerHTML=e.body;"undefined"===typeof c.getElementsByClassName&&(t=!1);e=[];var g=[],f=[],a=[],d=0;if(t)for(c=c.getElementsByClassName("tweet");d<c.length;){0<c[d].getElementsByClassName("retweet-credit").length?a.push(!0):a.push(!1);if(!a[d]||a[d]&&x)e.push(c[d].getElementsByClassName("e-entry-title")[0]),g.push(c[d].getElementsByClassName("p-author")[0]),f.push(c[d].getElementsByClassName("dt-updated")[0]);d++}else for(c=m(c,"tweet");d<c.length;)e.push(m(c[d],"e-entry-title")[0]),
    g.push(m(c[d],"p-author")[0]),f.push(m(c[d],"dt-updated")[0]),0<m(c[d],"retweet-credit").length?a.push(!0):a.push(!1),d++;e.length>j&&(e.splice(j,e.length-j),g.splice(j,g.length-j),f.splice(j,f.length-j),a.splice(j,a.length-j));c=[];d=e.length;for(a=0;a<d;){if("string"!==typeof s){var b=new Date(f[a].getAttribute("datetime").replace(/-/g,"/").replace("T"," ").split("+")[0]),b=s(b);f[a].setAttribute("aria-label",b);if(e[a].innerText)if(t)f[a].innerText=b;else{var k=document.createElement("p"),l=document.createTextNode(b);
    k.appendChild(l);k.setAttribute("aria-label",b);f[a]=k}else f[a].textContent=b}q?(b="",p&&(b+='<div class="user">'+v(g[a].innerHTML)+"</div>"),b+='<p class="tweet">'+v(e[a].innerHTML)+"</p>",n&&(b+='<p class="timePosted">'+f[a].getAttribute("aria-label")+"</p>")):e[a].innerText?(b="",p&&(b+='<p class="user">'+g[a].innerText+"</p>"),b+='<p class="tweet">'+e[a].innerText+"</p>",n&&(b+='<p class="timePosted">'+f[a].innerText+"</p>")):(b="",p&&(b+='<p class="user">'+g[a].textContent+"</p>"),b+='<p class="tweet">'+
    e[a].textContent+"</p>",n&&(b+='<p class="timePosted">'+f[a].textContent+"</p>"));c.push(b);a++}if(null==u){e=c.length;g=0;f=document.getElementById(w);for(d="<ul>";g<e;)d+="<li>"+c[g]+"</li>",g++;f.innerHTML=d+"</ul>"}else u(c);r=!1;0<h.length&&(twitterFetcher.fetch(h[0].id,h[0].domId,h[0].maxTweets,h[0].enableLinks,h[0].showUser,h[0].showTime,h[0].dateFunction,h[0].showRt,h[0].customCallback),h.splice(0,1))}}}();
    twitterFetcher.fetch('removed', '', 2, true, false, true, '', false, handleTweets);
    function handleTweets(tweets){
        var x = tweets.length;
        var n = 0;
        var element = document.getElementById('twitter');
        var html = '';
        while(n < x) {
          html += tweets[n];
          n++;
        };
        element.innerHTML = html;
    }
    function fbInit() {
        window.fbAsyncInit = function() {
        // init the FB JS SDK
        FB.init({
        appId      : '323684811110258',                     // App ID from the app dashboard
        channelUrl : '//gull.dev:59635/channel.html',       // Channel file for x-domain comms
        status     : true,                                  // Check Facebook Login status
        xfbml      : true                                   // Look for social plugins on the page
        });

        // Additional initialization code such as adding Event Listeners goes here
        };

        // Load the SDK asynchronously
        (function(d, s, id){
        var js, fjs = d.getElementsByTagName(s)[0];
        if (d.getElementById(id)) {return;}
        js = d.createElement(s); js.id = id;
        js.src = "//connect.facebook.net/en_UK/all.js";
        fjs.parentNode.insertBefore(js, fjs);
        }(document, 'script', 'facebook-jssdk'));
    }
    function fbPosts(){
        setTimeout(function(){  
            FB.api('/webantic/posts?access_token=removed', { limit: 2, fields : "message"  }, function(response) {
                console.log(response);
                var x = response.data.length;
                var n = 0;
                var element = document.getElementById('facebook');
                var html = '';
                while(n < x) {
                  html += "<p class='facebook-post'>" + response.data[n].message + '</p>';
                  n++;
                };
                element.innerHTML = html;
            }); 

        }, 750);
    }
    function mapInit(target){
        target.gmap({'zoom':15, 'center': '53.479465,-2.245141'}).bind('init', function(ev, map) {
            target.gmap('addMarker', {'position': '53.479591,-2.245527', 'bounds': false, 'icon' : '/assets/img/marker.png'}).click(function() {
                target.gmap('openInfoWindow', {'content': '<p style="margin-left: 10px; margin-right: 30px; margin-bottom: 0; color: #1e1e1e; font-size: 15px; font-weight: 300;"><strong>Manchester Town Hall</strong><br/>Albert Square,<br/>Manchester<br/>M60 2LA<br/>0161 234 5000<p>'}, this);
            });
        });
    }
    function getDirected(postcode){
        if (postcode != "") {
            $("#venue-map").gmap('destroy');
            $("#venue-map").gmap().bind('init', function(evt, map) { 
            $("#venue-map").gmap('displayDirections', { 'origin': postcode, 'destination': '53.479665,-2.245535', 'travelMode': google.maps.DirectionsTravelMode.DRIVING }, { 'panel': document.getElementById('verbose') }, function(result, status) {
                    if ( status === 'OK' ) {
                            $("body,html").animate({
                                scrollTop: 0
                            }, 500);
                    }
            });                                                                                                                                                                                                                       
            });
        } else {
            alert("please enter a valid postcode or placename");
        }
    }
    function canvasLoader(){
        var cl = new CanvasLoader('canvasloader-container');
        cl.setColor('#ffffff');
        cl.setShape('spiral');
        cl.setDiameter(200); 
        cl.setDensity(160); 
        cl.setRange(0.7);
        cl.setSpeed(1);
        cl.setFPS(60); 
        cl.show();
    }
    function doneLoading(){
        $('#canvasloader-container').fadeOut();
        setTimeout(function(){
            $('#canvasloader-container').addClass("hidden");
        }, 500);
    }
    function custDetails(){
        $("#return").val("http://gull.dev:51122/payment-complete?id=07875&"+"name="+encodeURIComponent($("#name").val())+"&email="+encodeURIComponent($("#email").val())+"&tel="+encodeURIComponent($("#tel").val())+"&quantity="+encodeURIComponent($("#guests").val()));
    }

});

function contact(options) {
    var
    //settings
    settings = $.extend( {
        //defaults
        "key" : "removed",
        "message" : "No Message",
        "subject" : "New Mail From Contact Form",
        "from_email" : "[email protected]",
        "from_name" : "Webantic",
        "to_email" : "[email protected]",
        "to_name" : "Webantic", 
        "success" : function(){}
    }, options),
    data = {
        "key" : settings.key,
        "message" : {
            "html" : settings.message,
            "subject" : settings.subject,
            "from_email" : settings.from_email,
            "from_name" : settings.from_name,
            "to" : [{
                "email" : settings.to_email,
                "name" : settings.to_name
            }]
        }
    };
    $.ajax({
        type: "POST",
        url: "https://mandrillapp.com/api/1.0/messages/send.json",
        data : data,
        success : settings.success()
    });
    return false;
    };
share|improve this question
add comment

1 Answer

up vote 2 down vote accepted

Is having all of my scripts in one "scripts.js" file like this good practice?

During development, no. You should break them into sensible chunks. For example, jQuery splits development of the library into chunks like "ajax", "core", "data" etc.

During production, you concatenate and uglify them, resulting to one small, mangled file that the browser will still understand.

Is my structure organised and logical or is there another best practice or way it could be substantially improved?

I suggest you take a look at RequireJS. It's a library that allows you to split your code into modules, and let modules load its "required modules" before running them.

// defining a module, maybe a module A
define([
  // which needs module B
  'path/to/modB'
],function(modB){
  //Run when B is loaded
});

So if moduleA needs code from moduleB, then you just state it as a dependency and the library will do the rest. That way, you won't worry about splitting up code and debate as to which should load first (or should appear as a script tag first).

RequireJS also has its own optimizer which is still basically a concat+minify operation.

Am I doing anything that will be significantly reducing the performance of my scripts and is there a way to rectify this?

I notice you cached .card-wrap but didn't use it. You are "fetching" it again with jQuery. Unless there has been changes to the old reference (like added .card-wraps, then you should use the cached version.

var cardWrap = $(".card-wrap"); ... verticalAlign($(".card-wrap"), $(".card-wrap img"));

You also have a lot of body event handlers sprinkled all over the place. I suggest you use a map instead of injecting them per event. Neater, and in one place.

//From:
$('body').on('event',fn1);
$('body').on('anotherevent',fn2);

//to    
$('body').on({
  event : fn1,
  anotherevent : fn2
});

The sticky menu is also a problem. Though you want it always visible, tapping to the scroll event is very costly. It fires several times per scroll. Also, you are fetching from the DOM everytime, generating a new jQuery object and recalculating the layout everytime.

Instead, I suggest a you cache .navbar.sticky.fixed.

// Keep sticky menu shrunk when off canvas menu is visible
var sticky = $(".navbar.sticky.fixed");
$(window).scroll(function(){
    if (bookForm === true) {
        sticky.css("left", -offCanvas.width());
    };
});

Also, if possible, use pure CSS to position things instead of JS. You can define class styles on your stylesheets and use JS to append classes. That way, CSS is reusable.

share|improve this answer
 
Brilliant advice, thanks. Some really useful bits of information in this answer. Require.js looks fantastic. –  Calvin Sep 6 '13 at 8:37
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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