HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Moving items around in a queue

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
arounditemsmovingqueue

Problem

What I'm trying to achieve here is sort queue of .item-dragable between different lists. Users can click on right and left arrows and items move in queue respectively, where the previous or next item gets placed in place of item that was moved. There is also a placeholder element that should not be counted as a valid sortable, so there are checks to ignore it. If an item is in the last list and is moved to the right, it moves to the first list. If it is in the first list and gets moved to the left it moves to last list (last item in last list, first item in first list).

The first thing that came to mind was switch statements, but I can't figure out a way to implement them in this case, as each check is unique. Could you please provide another alternative or let me know if there is any other better way than using so many if / else statements in this case?

There are more bits to this code that find correct adjacent lists, re initiate some stuff etc., but adding these in would make question very lengthy.

```
if ( (item.is(':last-child') || item.next().hasClass('item-dragable-placeholder') ) && direction == "right") {
var itemListAdjTmp = itemListAdj.find('.item-dragable').first(),
itemPartner;
if(itemList.find('.item-dragable').length <= 1) {
itemPartner = item.next('li');
item.insertBefore(itemListAdjTmp);
itemListAdjTmp.insertBefore(itemPartner);

} else {
itemPartner = item.prev('.item-dragable');
item.insertBefore(itemListAdjTmp);
itemListAdjTmp.insertAfter(itemPartner);
}
}
else if (!item.is(':last-child') && !item.next().hasClass('item-dragable-placeholder') && direction == "right") {
item.insertAfter(item.next('.item-dragable'));
}
else if (item.is(':first-child') && direction == "left") {
var itemListAdjTmp = itemListAdj.find('.item-dragable').last(),
itemPartner;
if(itemList.find('.ite

Solution

Here's some rewriting to remove the verbose nested if/else statements and some of the duplicate code.

First thing is to declare the variables on the first two lines outside of the if/else statements, since they don't change (and you'd just be repeating yourself).

The first one, itemPartner essentially uses an if/else in it's declaration.

Roughly translated:


[is this condition met] ? [yes] : [no]

It's just a shorter way of handling that if/else bit to declare that variable.

The second is the group of elements you pull the first and last out of. Since the group doesn't change, I figured we could declare it outside of the if/else blocks, and get the first and last within the blocks.

var itemPartner = (itemList.find('.item-dragable').length <= 1) ? item.next('li') : item.prev('.item-dragable'),
    itemListAdjTmp = itemListAdj.find('.item-dragable');

if(direction == "right"){
    if (item.is(':last-child') || item.next().hasClass('item-dragable-placeholder')) {
        item.insertBefore(itemListAdjTmp.first());
        itemListAdjTmp.first().insertAfter(itemPartner);
    }
    else if (!item.is(':last-child') && !item.next().hasClass('item-dragable-placeholder')) {
        item.insertAfter(item.next('.item-dragable'));
    }
}else if (direction == "left") {
    if (item.is(':first-child')) {
        item.insertAfter(itemListAdjTmp.last());
        itemListAdjTmp.last().insertBefore(itemPartner);
    }
    else{
        item.insertBefore(item.prev('.item-dragable'));
    }
}


I could take it a step further, and write it as I tend to prefer, with most variables declared up front. To me, this is easier to read and make sense of, since the variable names in the if/else statements clearly indicate what they refer to in only one word (instead of a line of code), but that may just be a personal preference.

To demonstrate:

var itemPartner = (itemList.find('.item-dragable').length <= 1) ? item.next('li') : item.prev('.item-dragable'),
    itemListAdjTmp = itemListAdj.find('.item-dragable'),
    isFirstChild = item.is(':first-child'),
    isLastChild = item.is(':last-child'),
    hasDragableClass = item.next().hasClass('item-dragable-placeholder');

if (direction == "right") {
    if (isLastChild || hasDragableClass) {
        item.insertBefore(itemListAdjTmp.first());
        itemListAdjTmp.first().insertAfter(itemPartner);
    }
    else if (!isLastChild && !hasDragableClass) {
        item.insertAfter(item.next('.item-dragable'));
    }
} else if (direction == "left") {
    if (isFirstChild) {
        item.insertAfter(itemListAdjTmp.last());
        itemListAdjTmp.last().insertBefore(itemPartner);
    }
    else {
        item.insertBefore(item.prev('.item-dragable'));
    }
}

Code Snippets

var itemPartner = (itemList.find('.item-dragable').length <= 1) ? item.next('li') : item.prev('.item-dragable'),
    itemListAdjTmp = itemListAdj.find('.item-dragable');

if(direction == "right"){
    if (item.is(':last-child') || item.next().hasClass('item-dragable-placeholder')) {
        item.insertBefore(itemListAdjTmp.first());
        itemListAdjTmp.first().insertAfter(itemPartner);
    }
    else if (!item.is(':last-child') && !item.next().hasClass('item-dragable-placeholder')) {
        item.insertAfter(item.next('.item-dragable'));
    }
}else if (direction == "left") {
    if (item.is(':first-child')) {
        item.insertAfter(itemListAdjTmp.last());
        itemListAdjTmp.last().insertBefore(itemPartner);
    }
    else{
        item.insertBefore(item.prev('.item-dragable'));
    }
}
var itemPartner = (itemList.find('.item-dragable').length <= 1) ? item.next('li') : item.prev('.item-dragable'),
    itemListAdjTmp = itemListAdj.find('.item-dragable'),
    isFirstChild = item.is(':first-child'),
    isLastChild = item.is(':last-child'),
    hasDragableClass = item.next().hasClass('item-dragable-placeholder');

if (direction == "right") {
    if (isLastChild || hasDragableClass) {
        item.insertBefore(itemListAdjTmp.first());
        itemListAdjTmp.first().insertAfter(itemPartner);
    }
    else if (!isLastChild && !hasDragableClass) {
        item.insertAfter(item.next('.item-dragable'));
    }
} else if (direction == "left") {
    if (isFirstChild) {
        item.insertAfter(itemListAdjTmp.last());
        itemListAdjTmp.last().insertBefore(itemPartner);
    }
    else {
        item.insertBefore(item.prev('.item-dragable'));
    }
}

Context

StackExchange Code Review Q#96208, answer score: 3

Revisions (0)

No revisions yet.