4
\$\begingroup\$

I'm looking for OOP tips or advice.

 ;(function ( ,ドル window, document, undefined ) {
 if ( typeof Object.create !== 'function' ) {
 Object.create = function (o) {
 function F() {}
 F.prototype = o;
 return new F();
 };
 }
 if (!Function.prototype.bind) { // check if native implementation available
 Function.prototype.bind = function(){ 
 var fn = this, args = Array.prototype.slice.call(arguments),
 object = args.shift(); 
 return function(){ 
 return fn.apply(object, 
 args.concat(Array.prototype.slice.call(arguments))); 
 }; 
 };
 }
 $.component = function( name, object ) {
 $.fn[name] = function( options ) {
 return this.each(function() {
 if ( !$.data( this, name ) ) {
 $.data( this, name, Object.create(object).init( 
 $.extend(true, {}, $.fn[name].opts, options), this ) 
 );
 }
 });
 };
 };
})( jQuery, window , document );
(function ( ,ドル window, document, undefined ) {
 var dropdown = {
 opts: {},
 init: function( options, el ) {
 this.$el = $(el).on('click.dropdown', 
 (function(evt){
 this.toggle(evt);
 }).bind(this)
 );
 this.opts = $.extend(true, this.opts, options);
 this.isShowing = false;
 return this;
 },
 show: function (evt) {
 if(evt) evt.preventDefault(); evt.stopPropagation();
 $(this.opts.header).addClass('show-mobile-nav');
 this.isShowing = true;
 this.bindClose();
 },
 hide: function (evt) {
 if(evt) evt.preventDefault();
 $(this.opts.header).removeClass('show-mobile-nav');
 this.isShowing = false;
 return this;
 },
 bindClose: function () {
 $('html').on('click.dropdown',
 (function(evt){
 var $targ = $(event.target || window.event.srcElement),
 targetIsNotDropdown = !$targ.is(this.opts.menu),
 targetsNotAChildOfDropdown = $(this.opts.menu).has( $targ ).length == 0;
 if( targetIsNotDropdown && targetsNotAChildOfDropdown) {
 $('html').off('.dropdown');
 this.hide();
 }
 }).bind(this)
 );
 return this;
 },
 toggle: function (evt) {
 if(evt) evt.preventDefault();
 if ( !this.isShowing ) {
 this.show(evt);
 } else {
 this.hide(evt);
 }
 }
 };
 $.component('dropdown',dropdown);
})( jQuery, window , document );
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked May 1, 2013 at 17:42
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

From a once over:

  • Your code is not consistently indented, especially the first dozen lines need some TLC
  • $.component = function( name, object ) { needs commenting, it is not clear what this is trying to achieve.
  • evt is an unfortunate parameter name, I would go for the classic e or the readable event
  • $targ is needlessly short, just use $target
  • You could probably use helper functions for if(evt) evt.preventDefault(); and if(evt) evt.preventDefault(); evt.stopPropagation();
  • Note that if(evt) evt.preventDefault(); evt.stopPropagation(); is exactly the kind of bug that makes people enforce {} around if blocks!
  • toggle() could use a ternary condition with !this.isShowing
  • 'show-mobile-nav' should probably not be hardcoded since you use it more than once, instead you should use a properly named variable

All in all, some pretty cool code.

answered Apr 1, 2014 at 0:58
\$\endgroup\$

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.