I have coded my own lightbox for the sake of my own custom needs because I want to be able to do this kind of thing myself and learn all the various things involved. I realize I could use a plugin, but that is not the point here.Last week I posted this vague question to get some feedback on the style, and ultimately the code I am posting here now is really a much better question I think:Is my method of making the lightbox an object filled with different independent methods valid?I like doing it this way because it allows me to make a bunch ofisolated individual functions that I can call on and reference whenneeded.When I need to modify part of the process, most things are isolatedand easy to work on.Sometimes I may have difficulty drawing the line between separatingfunctions and keeping them combined for simplicities sake.As you look at the code you might want to say Why didn't you do this, or why didn't you do that Or you might say Why are you doing it like this to which I will say, that will not be constructive for me. I am doing it this way because I have found it to be a nice way of doing things and frankly I am not as adept at writing the code other ways. I have been on a JS/Jquery crash course for about two years now trying to make the best I can of it.var lightbox={      start: function(){          $('body').on('click','img',function(){              var target = $(this).attr('src');              lightbox.createScreen();              lightbox.getDimensions();              lightbox.createImageBackDrop(target);              lightbox.listenForClose();          });      },      createScreen: function(){              var $screenElement = $('<div></div>').attr('id','modalscreen')               .css(                     {                       position:'absolute',                       width:'100%',                       height:'100%',                       zIndex:10000,                       backgroundColor: '#000000',                       opacity: '0.8'                                                          }                     )                lightbox.prependScreen($screenElement);        },       prependScreen: function($e){                 ($e).prependTo(document.body);       },       createImageBackDrop: function($image){          var $backDrop = $('<div></div>').attr('id','imageback')              lightbox.sizeBackDrop($backDrop,$image);       },       sizeBackDrop: function($e,src){           //original height / original width x new width = new height           var width = lightbox.getDimensions('x');           var imgWidth = Math.round(width*.52);           var imgHeight = Math.round(750/1000*imgWidth);           var bdWidth = imgWidth+20;           var bdHeight = imgHeight+20;           var bdMarginTop = -Math.abs(bdHeight/2);           var bdMarginLeft = -Math.abs(bdWidth/2);             $e.css(                    {                         position:'absolute',                         top:'50%',                         left:'50%',                         marginTop:bdMarginTop,                         marginLeft:bdMarginLeft,                         width: bdWidth+'px',                         height:bdHeight+'px',                         zIndex:10001,                         backgroundColor: '#FFF',                         padding:'10px'                    }                 )                lightbox.prependBackDrop($e);                lightbox.insertImage(src,imgWidth,imgHeight);       },       insertImage: function(src,w,h){            var imgMarginTop = -Math.abs(h/2);            var imgMarginLeft = -Math.abs(w/2);            var $image = $('<img>')                  .attr({                        'src':src,                        'height':h,                        'width':w                        })                  .css({                     position:'absolute',                     top:'50%',                     left:'50%',                     marginTop:imgMarginTop,                     marginLeft:imgMarginLeft,                 });          ($image).appendTo('#imageback');       },       prependBackDrop: function($e){          ($e).prependTo(document.body);       },       getDimensions: function(e){           var width = screenwidth.checkSize();           var height = screenwidth.checkHeight();           if(e == 'y'){               return height;           }           else if (e == 'x'){               return width;           }       },       listenForClose: function(){           $('body').on('click','#modalscreen',function(){           $('#modalscreen').remove();           $('#imageback').remove();           });       }}//end lightbox$(document).ready(screenwidth.start); //another object that returns screen size info$(document).ready(lightbox.start);
jQuery lightbox
javascript;jquery
First of all, this may be a matter of opinion (my CTO totally disagrees on this, for instance), but I think splitting a long method / function into tiny ones with a single, simple purpose is the right way of doing things. Actually, this is also the position held by the Clean Code book and many people out there.The benefits you state do exist, and other reasons to code this way can be found in aforementioned book. What i really like about this though :your code becomes self-documenting, so that it's easier for other people to understand what you intend to do. it helps to get an overview of a complex process without having to deal with all the low-level details, except if you want it. Perhaps more importantly, it helps scoping things (for example, in long functions you sometimes have to crawl all the way up because you don't remember what this or that var is, in which state it is, etc.). I also find it helps reasoning about the problem you're dealing with, because with this method you tend to split the problem into smaller problems, which is about the very essence of programming.That said, I have a few pointers that you might consider helpful.Use closuresYour lightbox object is not really necessary, and sometimes it obfuscates the code a bit.In fact, as your script only evaluates once (no subsequent call after start), you could replace your object with a closure : (function(){    // initialize stuff })();What happens here is that you create a function that you immediately call ; all vars in the function body are in a local scope that is not accessible from the outside (this prevents conflicts with other scripts). Using a closure, you will be able to run intialization code without having to stuff all your functions inside an object or worrying to pollute the global namespace :// the bang! is just another way to create a closure... // with one less character to type (closing parens)!function(){  // I like to put init stuff on top, this is debatable  doThis();  doThat();  doThose();  // these functions are local to the scope ;  // they are also callable from anywhere in this scope   // (search 'javascript variable lifting' for more info)  function doThis(){};   function doThat(){};  function doThose(){};}();Use closure-local variablesIn your code, your init stuff is called every time the user clicks on an image. This is bad because, for instance, you call createScreen every time, creating a new element an appending it to body over and over. As you will only need one screen throughout the entire app lifecycle, it would be sensible to do something like this : !function(){  // we will only create this once, and operate on this only element  var screen = createScreen();  function createScreen(){     //... create your element   };  function showScreen(){    screen.prependTo(document.body);  };  // remove() only removes the element from the DOM,  // it remains usable and accessible through the var  function hideScreen(){    screen.remove();  }}();You can even get a bit more object-oriented on this one :!function(){  // ad-hoc object, could be a real class with elaborate   // constructor and prototype  var screen = {    element: createScreen(),    show: function(){ this.element.prependTo(document.body); },    hide: function(){ this.element.remove(); }  };  $(document)    .on('click','img', open)    .on('click','#modalscreen', close);  function open(){    screen.show();    // insert image and stuff  };  function close(){    screen.hide();  };}();same method applies to other parts of your script (for instance, only create the modal once, then show/hide/resize it, and change image inside).Style considerationsJust a last minor remark : when declaring many vars, you can do :var foo = 'foo',    bar = 'bar',    baz = 'baz';It saves typing and it is a bit more clear. In fact, some recommend using the var keyword only once and on top of the scope.