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.
1 Answer 1
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.
-
\$\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\$evu– evu2015年11月02日 08:55:09 +00:00Commented 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 theinitializers
configuration object. \$\endgroup\$76484– 764842015年11月03日 01:07:05 +00:00Commented 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\$76484– 764842015年11月03日 01:07:11 +00:00Commented Nov 3, 2015 at 1:07