1
\$\begingroup\$

I have developed a jQuery Responsive Carousel. When opened, the carousel in the browser and the Chrome task manager, I see that the memory usage of my page is higher than the rest of the opened tabs (also comparing to the Gmail tab). But there is one more tab on the task manager, 'javascript memory', which shows that the page memory usage is lower compared to other tabs.

Can someone please review my code (also here) and suggest any improvements needed?

(function (factory) {
    if (typeof define === "function" && define.amd) {
        define(["jquery"], factory);
    } else if (typeof exports === "object") {
        module.exports = factory(require("jquery"));
    } else {
        factory(jQuery);
    }
})(function($){
	
	$.fn.jRCarousel = function(options){
		var _defaults = {
				width : 1349,				/* largest allowed width */
				height: 668,				/* largest allowed height */
				slides : [], 				/* array of images source or gets slides by 'slide' class */
				slideLayout : 'fill', 		/* contain | fill | cover */
				animation: 'slide', 		/* slide | scroll | fade | zoomInSlide | zoomInScroll */
				animationSpeed: 700,
				animationInterval: 2000,
				autoplay: true,
				controls: true,
				navigation: null, 			/* circles | squares */
				onSlideShow: function(){}   /* callback when Slide show event occurs */
		}
		var _settings = $.extend( true, {}, _defaults, options );
		var _container = this;
		var _width = _settings.width;
		var _height = _settings.height;
		var _aspectRatio = _settings.width/_settings.height;
		var _wrapper = $( "<div class='jRCarousel' />" ).css({ display:'block', position: 'relative', overflow: 'hidden', width: '100%', height: '100%' })
														.appendTo(_container);
		var _currentSlide;
		var _targetSlideIndex;
		var _animations = new Animations();
		var _previousButton;
		var _nextButton;
		var _timer;
		
		(function setup(){
			
			/* create jRCarousel */
			createjRCarousel();
			
			/* create control buttons */
			if(_settings.controls){
				_createControls();
			}
			
			/* create navigation bar */
			if(_settings.navigation){
				_createNavigation();
			}
			
			/* start jRCarousel if autoplay */
			if(_settings.autoplay){
				_playjRCarousel();
			}
			
			/* adjust size according to device */
			addEventListener('resize', _makeResposive);
			_makeResposive();
			
			function createjRCarousel(){
				/* create jRCarousel stack, and keep first slide at top of stack */
				if(_settings.slides.length){
					for(var i = 0;  i < _settings.slides.length; i++){
						var slide = $( "<img class='slide' data-index="+i+" />" )
						.prop({src:_settings.slides[i].src, alt:"'"+_settings.slides[i].src+"'"}).detach();
						_wrapper.append(slide);
					}
				}else{
					_container.find('.slide').each(function(i){
						var slide = $(this).attr('data-index', i).detach();
						_wrapper.append(slide);
					});
				}
				_wrapper.find('.slide').css({position: 'absolute', left: 0, top:0, width:'100%', height:'100%', objectFit:_settings.slideLayout, backgroundColor:'#fff'});
				_wrapper.find('.slide img').css({ width:'100%', height:'100%', objectFit:_settings.slideLayout });
				
				_currentSlide = _wrapper.find('.slide').first().detach();
				_wrapper.append(_currentSlide);
				_container.css({width: _width+'px', height: _height+'px' });

			}
			
			function _createControls(){
				_previousButton = $( "<div class='previous controls' style='left: 9px; transform: rotate(-45deg);'></div>");
				_nextButton = $( "<div class='next controls' style='right: 9px; transform: rotate(135deg);'></div>");
				_previousButton.add(_nextButton).appendTo(_wrapper)
							   .css({position: 'absolute', top:'42%', zIndex:1, display: 'inline-block', padding: '18px', boxShadow: '7px 7px 0 1px #777 inset', cursor:'pointer',opacity:'0.7'})
							   .hide();
				
				 /* event handlers */
				_previousButton.on('click', function(){
					_wrapper.find('.controls').hide();
					_startCarousel(_getPreviousSlide());
				});
				_nextButton.on('click', function(){
					_wrapper.find('.controls').hide();
					_startCarousel(_getNextSlide());
				});
				
				/* event handlers */	
				_container.hover(function(){
					_wrapper.find('.controls').fadeIn();
				},function(){
					_wrapper.find('.controls').fadeOut();
				});
			}
			
			function _createNavigation(){
				var type = _settings.navigation;
				var _navigation = $('<div class=navigation />').css({ textAlign: 'right' });
				for(var i = 0;  i < _wrapper.find('.slide').length; i++){
					_navigation.append('<div class=nav></div>');
				}
				if(type == 'circles'){
					_navigation.find('.nav').css({ border: '0 dashed #ccc', borderRadius: '12px' });
				}else if(type == 'squares'){
					_navigation.find('.nav').css({ border: '0 dotted #fff' });
				}
				_navigation.find('.nav').css({ display: 'inline-block', margin: '4px', cursor: 'pointer', backgroundColor: '#777', width: '12px', height: '12px' })
										.first().css({ borderWidth: '2px' });
				_wrapper.after(_navigation);
				
				 /* event handler */
				_container.on('click', '.nav', function(){
					_startCarousel(_getSlideByIndex($(this).index()));
				})
				.css({ marginBottom: '24px' });
			}
			
			function _playjRCarousel(){
				_timer = setInterval(_startCarousel, _settings.animationInterval+_settings.animationSpeed);
				_wrapper.find('.controls').hide();
				
				/* event handlers */	
				_container.hover(function(){
					clearInterval(_timer);
					_timer = 0;
					_wrapper.find('.controls').fadeIn();
				},function(){
					_wrapper.find('.controls').fadeOut();
					_timer = setInterval(_startCarousel, _settings.animationInterval+_settings.animationSpeed);
				});
			}
			
			jQuery.fx.interval = 7;
			
		})();
		
		function _getPreviousSlide(){
			return _wrapper.find('.slide').eq(-2);
		}
		function _getCurrentSlide(){
			return _wrapper.find('.slide').last();
		}
		function _getNextSlide(){
			return _wrapper.find('.slide').first();
		}
		function _getSlideByIndex(idx){
			return _wrapper.find('.slide[data-index='+idx+']');
		}
		
		function Animations(){
			this.animations = {
					slide : _slide,
					scroll: _scroll,
					fade: _fade,
					zoomInSlide: _zoomInSlide,
					zoomInScroll: _zoomInScroll
				}
		}
		Animations.prototype.run = function run(animation, direction){
				this.animations[animation](direction);
		}
		
		function _startCarousel(slide){
			var targetSlide = slide || _getNextSlide();
			_currentSlide = _getCurrentSlide();
			
			_targetSlideIndex = targetSlide.data('index');
			var currentSlideIndex = _currentSlide.data('index');
			
			/* same slide */
			if(currentSlideIndex == _targetSlideIndex){
				return -1;
			}
			
			/* show current & target slide and hide all other */
			_wrapper.find('.slide').not(_currentSlide[0]).hide();
			targetSlide.show();
			
			/* target is in next slides */
			if(currentSlideIndex < _targetSlideIndex){
				/* get next slide & make it to appear on top of stack to run animation */
				while(_getCurrentSlide().data('index') != _targetSlideIndex){
					_currentSlide = _getNextSlide().detach().appendTo(_wrapper);
				}
				_animations.run(_settings.animation, 1);
			}
			/* target is in previous slides */
			else{
				/* get previous slide, run animation & make target slide to appear on top of stack after animation */
				_animations.run(_settings.animation, -1);
			}
			
			/* set active nav icon */
			_container.find('.nav').css({ borderWidth: 0 })
								   .eq(_targetSlideIndex).css({ borderWidth: '2px' });	
		}
		
		function _stopCarousel(){
			/* If continuous slideShow is not in progress */
			if(!_timer){
				_wrapper.find('.controls').show();
			}
			_settings.onSlideShow.call(this, _currentSlide);
		}
		
		/* direction is 1 = next & -1 = previous */
		function _slide(direction){
			if(direction == 1){
				_currentSlide.css({ left:_width+'px' })
				.animate({
					left: 0
				},
				{
					duration: _settings.animationSpeed,
					complete: function(){	
						_stopCarousel();
					}
				})
			}else{
				_currentSlide
				.animate({
					left: _width+'px'
				},
				{
					duration: _settings.animationSpeed,
					complete: function(){
						_currentSlide.css({ left: 0 });
						/* make target slide to appear on top of stack after animation */
						while(_getCurrentSlide().data('index') != _targetSlideIndex){
							_currentSlide = _getCurrentSlide().detach().prependTo(_wrapper);
						}
						_currentSlide = _getCurrentSlide();
						_stopCarousel();						
					}
				})
			}
		}
		
		function _scroll(direction){
			if(direction == 1){
				_currentSlide.css({	top: _height+'px' })
				.animate({
					top: 0
				},
				{
					duration: _settings.animationSpeed,
					complete: function(){
						_stopCarousel();
					}
				})
				
			}else{
				_currentSlide
				.animate({
					top: _height+'px'
				},
				{
					duration: _settings.animationSpeed,
					complete: function(){
						_currentSlide.css({ top: 0 });
						/* make target slide to appear on top of stack after animation */
						while(_getCurrentSlide().data('index') != _targetSlideIndex){
							_currentSlide = _getCurrentSlide().detach().prependTo(_wrapper);
						}
						_currentSlide = _getCurrentSlide();
						_stopCarousel();
					}
				})
			}
		}
		
		function _fade(direction){
			if(direction==1){
				_currentSlide.css({	opacity: 0 })
				.animate({
					opacity: 1
				},
				{
					duration: _settings.animationSpeed,
					complete: function(){
						_stopCarousel();
					}
				})
			}else{
				_currentSlide
				.animate({
					opacity: 0
				},
				{
					duration: _settings.animationSpeed,
					complete: function(){
						_currentSlide.css({ opacity: 1 });
						/* make target slide to appear on top of stack after animation */
						while(_getCurrentSlide().data('index') != _targetSlideIndex){
							_currentSlide = _getCurrentSlide().detach().prependTo(_wrapper);
						}
						_currentSlide = _getCurrentSlide();
						_stopCarousel();
					}
				})
			}
		}
		
		function _zoomInScroll(direction){
			if(direction==1){
				_currentSlide.css({	height: 0 })
				.animate({
					height: '100%'
				},
				{
				duration: _settings.animationSpeed,
				complete: function(){
					_stopCarousel();
				}
			})
			}else{
				_currentSlide
				.animate({
					height: 0
				},
				{
					duration: _settings.animationSpeed,
					complete: function(){
						_currentSlide.css({ height:'100%' });
						/* make target slide to appear on top of stack after animation */
						while(_getCurrentSlide().data('index') != _targetSlideIndex){
							_currentSlide = _getCurrentSlide().detach().prependTo(_wrapper);
						}
						_currentSlide = _getCurrentSlide();
						_stopCarousel();
					}
				})
			}
		}
		
		function _zoomInSlide(direction){
			if(direction==1){
				_currentSlide.css({
					width: 0,
					left: _width+'px'
				})
				.animate({
					width: '100%',
					left:0
				},
				{
				duration: _settings.animationSpeed,
				complete: function(){
					_stopCarousel();
				}
			})
			}else{
				_currentSlide
				.animate({
					width: 0
				},
				{
					duration: _settings.animationSpeed,
					complete: function(){
						_currentSlide.css({ width:'100%' });
						/* make target slide to appear on top of stack after animation */
						while(_getCurrentSlide().data('index') != _targetSlideIndex){
							_currentSlide = _getCurrentSlide().detach().prependTo(_wrapper);
						}
						_currentSlide = _getCurrentSlide();
						_stopCarousel();
					}
				})
			}		
		}
		
		function _makeResposive(){
			_container.width('100%');
			_width = _container.width() < _settings.width ? _container.width() : _settings.width;
			_height = _width/_aspectRatio;
			_container.css({width: _width+'px', height: _height+'px' });
		}	
		
		/* public API */
		this.showSlide = function(index){
			_startCarousel(_getSlideByIndex(index));
		}
		this.getCurrentSlide = function(){
			return _getCurrentSlide();
		}
		this.getSlideByIndex = function(index){
			return _getSlideByIndex(index);
		}
		this.showPreviousSlide = function(){
			_startCarousel(_getPreviousSlide());
		}
		this.showNextSlide = function(){
			_startCarousel(_getNextSlide());
		}

		return this;
	}
	
});

\$\endgroup\$
1
\$\begingroup\$

Presumably the function (factory) {...} bit at the top is robust. I've not encountered it before.

A few things strike me in the plugin - not necessarily to do with memory consumption.

  1. Where possible, avoid the dynamic creation of inner functions in favour of named functions in an outer scope. Chief candidates are the animations' "complete" callbacks.
  2. Avoid unnecessary assignments. var foo = $(...); _wrapper.append(foo); will simplify to $(...).appendTo(_wrapper);
  3. Avoid unnecessary detach(). Methods .append(), appendTo(), prepend(), .prependTo() will move elements without needing to detach them first. eg $(...).detach().appendTo(...) will simplify to $(...).appendTo(...).
  4. The Animations object and its instance _animations seem to be a slightly expensive way to do something very simple. The inner plain object animations would do the job on its own, with the two .run() calls longhanded out.
  5. With jQuery on board, why code addEventListener('resize', _makeResposive) in POJS? Are there not cross-browser issues?
  6. Some of the outer vars are unnecessary. eg _previousButton, _nextButton can be localized within _createControls(), and _width, _height, _aspectRatio can be omitted as _settings is always in scope wherever those vars are used.
  7. It's not necessary to have one set of defaults per instance of the carousel, therefore _defaults can be moved into the outermost scope above $.fn.jRCarousel = function(options) {.
  8. Does alt: "'" + _settings.slides[i].src + "'" really need the quotes?
\$\endgroup\$
  • \$\begingroup\$ function (factory) {...} is UMD pattern that I came across while publishing the package to npm. 2. nice catch. 3. I really didnt know that, that's very stupid of me. 4. I had that just for modularity. 5. I was targeting Internet explorer 9+, so it was ok, but you are right could have used jquery for this. \$\endgroup\$ – vinayakj Nov 30 '15 at 21:53
  • \$\begingroup\$ 6. _previousButton, _nextButton I have used these references in methods providing public API, which are in outer scope of _createControls, and for _aspectRatio I thought I should compute it once and use later. You are right _width, _height should have been omitted, actually I was thinking of making small vars to reduce file size & increase readability. 7. I have taken the code from Jquery site, to startup, didnt think much. point noted. 8. Yes.. ' quotes are needed.. If user has a alt property which contains space in between words. \$\endgroup\$ – vinayakj Nov 30 '15 at 21:53
  • \$\begingroup\$ @vinayakj, thank you for your feedback. Good to know that some of my points were useful. And I have learned a few things too. Thank you in particular for the UMD link, which is the sort of thing I should have read before! Good luck with your project. \$\endgroup\$ – Roamer-1888 Nov 30 '15 at 23:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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