4
\$\begingroup\$

I've written a function that moves selected items from a list into a new position. I've found that in order to do this, I need to retain the original 'index structure' of the array. The way I've done this is, instead of splicing the array right away, I set the elements to null, then later traverse the list backwards and splice the null elements. I don't know if this is the best way of doing this, but I thought I would post it here to see if anyone else has an idea of how to better accomplish this.

https://jsfiddle.net/g407pjbk/1/

function drag_and_drop(list, before_index){
    const selected = [];

    for(let index in list){
        const item = list[index];

        if(item.selected){
            selected.push(item);

            // Set elements to null in order to retain indecies
            list[index] = null;
        }
    }

    // Insert selected items
    list.splice(before_index, 0, ...selected);

    // Backwards traverse and splice remaining null elements
    for(let i=list.length-1;i>=0;i--){
        if(list[i] == null)
            list.splice(i, 1);
    }

    return list;
}
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

So you don't need two loops here. You can just use one loop to

  1. First, get the selected items
  2. Remove the selected items from the actual array. So you can later put them in the index you want.

So to do that, first, you have to retrieve the indices where the selected obj is there. Then spice it. IN the loop

for (let index in list) {
    const item = list[index];

    if (item.selected) {
        selected.push(item);
        list.spice(index,1)
    }
}

So you don't have to run another loop to remove the null values. You can now simply just put the values using another spice. That saves you a loop.

You shouldn't be using for ... in for the reasons you have got in RoToRa's answer.

My approach: I prefer using just one filter. In that, you can do the same thing.

function drag_and_drop(list, before_index) {
  const selected = list.filter((obj,i) => obj.selected ? list.splice(i,1) : obj.selected)
  list.splice(before_index - selected.length , 0, ...selected)
  return list
}

So here, first we check if the object is selected and if it is then we'll just splice it off and if we didn't got a selected item then we can just return the selected item in the selected loop.

So now you have inside selected, the values which have item.selected as true and your list don't have them now because you did splice already in the filter, so you can just splice them with another splice.

Now the index is not anymore before_index it's two values more, because we took out two objects. So to balance that, we will do before_index - selected.length because that's the number of indexes we are going to drop.

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

First some general remarks to coding conventions:

  • JavaScript uses camelCase identifiers, not snake_case.
  • Insert a space between keywords and a following opening bracket (if (...) so that they don't look like functions.

I'm not a big fan of the method name. It should be called after what is does, not what it's for. moveSelectedItemsBeforeIndex would be my choice.


Never use for ... in on arrays. It iterates over all properties not only integer properties, and it's not guaranteed to do that in order. Use either a regular for loop or .forEach().


Always use === for comparison, unless you specifically want loose equality (null === undefined is false, but null == undefined is true).


You can replace the final loop with a simple .filter:

return list.filter(i => i !== null);

An alternative implementation with a more functional approach could be:

function drag_and_drop(list, before_index){
    let part1 = list.slice(0, before_index);
    let part2 = list.slice(before_index);

    let [selected1, unselected1] = groupBySelect(part1);
    let [selected2, unselected2] = groupBySelect(part2);

    return [...unselected1, ...selected1, ...selected2, ...unselected2];
}

function groupBySelect(list) {
    return list.reduce(
        (acc, item) => item.selected ? [[...acc[0], item], acc[1]] : [acc[0], [...acc[1], item]], 
        [[], []]
    );
}

It first splits at the index and then groups the elements of the two "halves" of the list whether they are selected or not and finally reassembles the array in the right order.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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