patternjavascriptMinor
Moving items around in a queue
Viewed 0 times
arounditemsmovingqueue
Problem
What I'm trying to achieve here is sort queue of
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
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
.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
The first one,
Roughly translated:
[is this condition met] ? [yes] : [no]
It's just a shorter way of handling that
The second is the group of elements you pull the
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
To demonstrate:
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.