3
\$\begingroup\$

As a very novice developer, I'm reading more about OOP and structuring JavaScript in a sensible way and have been using the following format for a little while now. It's difficult to know the best way to do things because most examples I see online are for much larger websites than I currently build. More like applications than websites, spanning multiple files and thousands of lines of code, where as mine is just in a single JS file spanning around 1k lines at most. I'm also using jQuery on each project and most things don't use a library like this, or they're based around Angular, etc.

jQuery(document).ready(function($){
 Site.init();
});
var Site = (function($) {
 // ================ !CACHING COMMON VARS
 // Aliased Variables
 var cfg, throttle, helper, loader; 
 // DOM caching
 var $win = $(window),
 $doc = $(document);
 // Globals
 var w = {
 width: $win.width(),
 height: $win.height(),
 scroll: $doc.scrollTop()
 }
 return {
 // ================ !SCOPE SETTINGS
 settings: {
 speed_fast: 300,
 speed_slow: 600,
 easing: [0.55, 0, 0.1, 1] // SwiftOut (Google Material Design)
 },
 // ================ !SITE
 init: function() {
 // Aliases
 cfg = this.settings;
 throttle = this.handlers.throttle;
 helper = this.helpers;
 loader = this.handlers.loader;
 // ================ !SITEWIDE FUNCTIONS
 // Form validation & ajax
 this.formProcess();
 // ================ !SPECIFIC FUNCTIONS
 // Share Popups
 helper.check('.js-social-share', this.sharePopup);
 },
 // ================ !EVENT-HANDLERS
 handlers: {
 // Throttle resize and scroll events
 throttle: function(handler, func) {
 var eventHappend = false,
 throttleOps = {};
 throttleOps[handler] = function(){
 eventHappend = true;
 }
 $win.on(throttleOps);
 setInterval(function() {
 if ( eventHappend ) {
 eventHappend = false;
 if (handler === 'resize') {
 window_width = $win.width();
 window_height = $win.height();
 }
 if (handler === 'scroll') {
 scroll_top = $doc.scrollTop();
 }
 func();
 }
 }, 250);
 },
 loader: function(func) {
 $win.on('load', func);
 }
 }, // End Handlers
 // ================ !HELPER/UTILITY FUNCTIONS
 helpers: {
 // Get a random Num between min and max
 getRandom: function(min, max) {
 return Math.floor(Math.random() * (max - min + 1)) + min;
 },
 // Check if element exists and run function
 check: function(el, func) {
 if ($(el).length) {
 func();
 }
 }
 }, // End Helpers
 // ================ !SITEWIDE FUNCTIONS
 // Open Share popups in new window
 sharePopup: function() {
 $('.js-social-share a').on('click', function(){
 if ($(this).attr('href').indexOf('http') === 0) {
 var newwindow = window.open($(this).attr('href'), '', 'height=450, width=700');
 if (window.focus) {
 newwindow.focus();
 }
 return false;
 }
 });
 }, // End sharePopup()
 // ================ !SPECIFIC FUNCTIONS
 // Process Forms.
 formProcess: function() {
 var $forms = $('.js-process-form');
 // Make sure forms are valid (using jQuery Validate)
 $forms.each(function(){
 $(this).validate({
 ignore: '',
 rules: {
 upload: {
 required: true
 }
 },
 onfocusout: function(element) {
 this.element(element);
 },
 onkeyup: false
 });
 });
 // On submit do Ajax post
 $forms.on('submit', function(e){
 e.preventDefault();
 var $this = $(this);
 if ($this.valid()) {
 // Uses site object from Worpress
 $.ajax({
 url: WP.base_dir+"/wp-admin/admin-ajax.php",
 type:'POST',
 data: $.extend({
 action: 'contact_email'
 }, $this.serializeObject()),
 dataType: 'json',
 success: function(data){
 //Access the returned JSON
 $this.html('<h1>' + data.message + '</h1>');
 }
 });
 }
 });
 }, // End formProcess()
 }; // End return Site
})(jQuery);

Main things that are bothering me are:

  • Are things enclosed correctly? (so as to not conflict with anything another dev might add in to the domready)
  • Are the event handlers/checkers even efficient?
  • Am I doing anything glaringly wrong?

There are a few things that are included elsewhere, so don't worry about something that is referenced but not in the script. I've included a couple of example functions in there as I would normally use them.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Oct 28, 2015 at 15:57
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

First, I would question why the Site object has so many properties in its API. It looks to me that Site.init is the only method that needs to be public and that all of the other methods/objects (Site.settings, Site.handlers, Site.helpers, Site.sharePopup, and Site.formProcess) are all intended to be used internally to the Site object implementation and do not need to be "public".

With that said, if Site exists only as an interface to a single method, why not simply replace the Site object with a function?

var initSite = function ($) {
 var settings, handlers, helpers, sharePopup, formProcess;
 /* Site initialization implementation goes here. */
};

And our ready handler would become:

jQuery(initSite);

On another note, I do not like the way that your selector strings are scattered around your code and are hard-coded with the feature implementations (sharePopup and processForm). I would recommend moving the selectors into a single configuration object and having your feature initializers take a selector or jQuery object as a parameter.

var initForms = function ($forms) {
 $forms.on('submit', function (e) {
 /* Do AJAX handling here. */
 e.preventDefault();
 });
};
var initSharePopup = function ($anchor) {
 $anchor.on('click', function (e) {
 /* Do Popup stuff here. */
 e.preventDefault();
 });
};
var initializers = {
 '.js-process-form': initForms,
 '.js-social-share a': initSharePopup
};

Any additional features we want to include in our pages can be built with init method similar to the ones above and implementing the same interface. Our site initialization will now be responsible for iterating through our initializers object and calling the init methods for selectors it finds in the document (essentially doing the work of your helper.check method).

$.each(initializers, function (selector, initializer) {
 var $els = $(selector);
 if ($els.length > 0) {
 initializer($els);
 }
});

This way, when we want to add features/modules to our page, we need only to add a selector, initializer pair to our initializers object, and our feature will get initialized on pages only when the corresponding selector is found.

answered Oct 31, 2015 at 5:26
\$\endgroup\$
3
  • \$\begingroup\$ Thanks 76484, I have a couple of questions regarding your reasoning though: Is it more performant as a function rather than an object? The object syntax seems more readable to me and there have been a couple of times where I've used Site functions outside of init. And what is the difference between your initialiser and the way I am doing it? Clearly it's different but is there performance gains? And a question about those initialisers, is it better to check for an element before running the function or does the function get stored in memory anyway? Thanks for the feedback! :) \$\endgroup\$ Commented Nov 2, 2015 at 8:55
  • \$\begingroup\$ I can't imagine there would be any noticeable performance gain either way. I am aiming for readability and maintainability with my solution. I find that your Site object is just too big and does too much for me to wrap my head around. I do think there are gains in readability and reusability in my version because each feature initializer is its own module. You could use the initializer method on all of your projects and you would need only to update the initializers configuration object. \$\endgroup\$ Commented Nov 3, 2015 at 1:07
  • \$\begingroup\$ The reason I check for an element before invoking an initializer with it is simply because it would be pointless to call the initializer with no element(s) to affect. \$\endgroup\$ Commented Nov 3, 2015 at 1:07

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.