Jquery sortable style function

Question

This is a basic sorting function written in jQuery that moves items in the DOM around to create empty spaces for a droppable(). Since there is so much going on in a droppable over event, i'd love some feedback on how this could be done better.

This is all in a function called on the over event of the droppable()

I have an array of empty spots

 var emptyHolders = [];
 $('.pipeline-holder').each(function(){
 if($(this).hasClass('holder-empty')){
 var eid = $(this).attr('id');
 emptyHolders.push(eid.substring(14));
 }
 });

cid is the droppable element that you're over top of, so I find the next open slot using

 for (var i = 0; i < emptyHolders.length; i++) {
 var currentEmpty = emptyHolders[i];
 if (currentEmpty > cid) {
 nextEmpty = currentEmpty;
 i = emptyHolders.length;
 } else {
 prevEmpty = parseInt(currentEmpty);
 }
 }

If there is a an empty slot further down the list, I use a for loop to loop through moving the items around the DOM to make the space needed to drop the element.

 if (nextEmpty != null) {
 var moveMe = nextEmpty -1;
 for (var i = moveMe; i >= cid; i--) {
 var nextcount = i + 1;
 var me = $('#pipeline-rank-' + i); 
 var next = $('#pipeline-rank-' + i).parents('.pipeline-rank-row').next().find('.pipeline-holder');
 var pid = $('#pipeline-rank-' + i).find('.content-wrapper').attr('id');
 next.append($('#pipeline-rank-' + i).find('.content-wrapper'));
 next.removeClass('holder-empty');
 next.siblings('.remember-my-position-hover').html(pid);
 }
 $('#pipeline-rank-' + cid).addClass('holder-empty');
 } 

If there is not one further down the list, I do the same thing in reverse to check if there is a spot above the droppable to push items up into.

It's usable here: http://jsfiddle.net/mstefanko/LEjf8/9/

Any thoughts are extremely appreciated!

Answer 1

Here you have some general thoughts:

1. First of all, cache jQuery elements (var $this = $(this); instead of multiple executions of $(this)). http://jsfiddle.net/tomalec/pXPdS/1/

2. You can also chain jQuery methods, it simplifies codes, and helps with above.

3. To reduce amount of code you can use toggleClass instead of addClass removeClass.http://jsfiddle.net/tomalec/pXPdS/2/

4. Instead of selecting $('#rankDrag') in start callback you can use reference given in arguments ui.helper http://jsfiddle.net/tomalec/pXPdS/3/

5. To get id of empty/not empty holder ids, in my opinion, .map is more readable than .each. Moreover, you can move that function outside rearrange to avoid re-declaring it every time.

   function getTruncatedId(index, element){
    return element.id.substring(14);
   }
   function rearrange(dropid) {
    //..
    emptyHolders = holders.filter('.holder-empty').map(getTruncatedId);
    isRanked = holders.filter(':not(.holder-empty)').map(getTruncatedId);
    //..
   }
   

   http://jsfiddle.net/tomalec/pXPdS/4/

6. Pass dropped element to rearrange, so you will not have to traverse through DOM again:

   rearrange( $(this).attr('id') );//dropId = $(this).attr('id');
   //..
   isEmpty = $('#'+dropId).hasClass('holder-empty');
   

   http://jsfiddle.net/tomalec/pXPdS/5/