将所选元素移动一个并返回其更新的索引

背景:

我最近反编译了我的一个老项目,它的源代码丢失了,我正在重构我多年前写的所有这些糟糕的代码。

我有一个列表,显示的项目由用户界面,项目可以上下移动,多选择是允许的。我有办法 int[] moveUp(int[] selectedIndices) 它更新模型并返回列表中移位元素的新索引(这样我就可以在模型更改后更新UI)。

selectedIndices 来自 JList#getSelectedIndices 这保证了排序顺序没有重复。

旧解决方案:

public int[] moveUp(int[] selectedIndices) {
    Action[] array = concatenate(new Action[]{null}, actions.toArray(new Action[0]));
    for (int i = 0; i < selectedIndices.length; i++) {
        swap(array, selectedIndices[i] + 1, selectedIndices[i]);
        selectedIndices[i] -= 1;
    }
    actions = new ArrayList<>(this.actions.size());
    for (Action action : array) {
        if (action != null) {
            actions.add(action);
        }
    }
    return selectedIndices;
}

问题:

如果我有行动 [a, b, c, d] 和 选择的骰子 是 [0, 1, 3] ,尽管结果是正确的( [a, b, d, c] ,返回的新索引是 [-1, 0, 2] ,而他们应该 [0, 1, 2] 。

新解决方案:

public int[] moveUp(int[] selectedIndices) {
    List<Action> selectedActions = stream(selectedIndices).mapToObj(actions::get).collect(toList());

    actions.add(0, null);
    for (int selectedIndex : selectedIndices) {
        swap(actions, selectedIndex + 1, selectedIndex);
    }
    actions.remove(null);

    return selectedActions.stream().mapToInt(actions::indexOf).toArray();
}

问题:

它既不干净,也不高效。

问题:

如何干净高效地实施?

MCVE和测试:

<properties>
    <maven.compiler.source>1.8</maven.compiler.source>
    <maven.compiler.target>1.8</maven.compiler.target>
</properties>

<dependencies>
    <dependency>
        <groupId>junit</groupId>
        <artifactId>junit</artifactId>
        <version>4.12</version>
        <scope>test</scope>
    </dependency>
    <dependency>
        <groupId>com.shazam</groupId>
        <artifactId>shazamcrest</artifactId>
        <version>0.11</version>
        <scope>test</scope>
    </dependency>
</dependencies>

/////////////////////////////////////////////////////////////////////////

public interface Action {

}

/////////////////////////////////////////////////////////////////////////

import java.util.ArrayList;
import java.util.List;

import static java.util.Arrays.stream;
import static java.util.stream.Collectors.toList;

public class Actions {

    private List<Action> actions = new ArrayList<>();

    public void add(Action action) {
        this.actions.add(action);
    }

    public int[] moveUp(int[] selectedIndices) {
        List<Action> selectedActions = stream(selectedIndices).mapToObj(actions::get).collect(toList());

        actions.add(0, null);
        for (int selectedIndex : selectedIndices) {
            swap(actions, selectedIndex + 1, selectedIndex);
        }
        actions.remove(null);

        return selectedActions.stream().mapToInt(actions::indexOf).toArray();
    }

    private static <T> void swap(List<T> list, int index, int index2) {
        T t = list.get(index);
        list.set(index, list.get(index2));
        list.set(index2, t);
    }

}

/////////////////////////////////////////////////////////////////////////

public class FakeAction implements Action {

    @SuppressWarnings("FieldCanBeLocal") // used by shazamcrest
    private final String name;

    private FakeAction(String name) {
        this.name = name;
    }

    static Actions actionsWithElements(int... elementsNames) {
        Actions actions = new Actions();
        for (int elementName : elementsNames) {
            actions.add(new FakeAction(String.valueOf(elementName)));
        }
        return actions;
    }

    static int[] indices(int... indices) {
        return indices;
    }

}

/////////////////////////////////////////////////////////////////////////

import org.junit.Test;

import static com.shazam.shazamcrest.MatcherAssert.assertThat;
import static com.shazam.shazamcrest.matcher.Matchers.sameBeanAs;
import static FakeAction.actionsWithElements;
import static FakeAction.indices;

public class ActionsMoveUpTest {

    @Test
    public void movesUpSingleAction() {
        Actions actions = actionsWithElements(0, 1, 2, 3);

        actions.moveUp(indices(2));

        assertThat(actions, sameBeanAs(actionsWithElements(0, 2, 1, 3)));
    }

    @Test
    public void movesUpMultipleActions() {
        Actions actions = actionsWithElements(0, 1, 2, 3);

        actions.moveUp(indices(1, 3));

        assertThat(actions, sameBeanAs(actionsWithElements(1, 0, 3, 2)));
    }

    @Test
    public void doesNothingWhenArrayOfIndicesToMoveUpIsEmpty() {
        Actions actions = actionsWithElements(0, 1, 2, 3);

        actions.moveUp(indices());

        assertThat(actions, sameBeanAs(actionsWithElements(0, 1, 2, 3)));
    }

    @Test
    public void doesNotLoseSelectionWhenMovingUpTheTopAction() {
        Actions actions = actionsWithElements(0, 1, 2, 3);

        int[] newIndices = actions.moveUp(indices(0));

        assertThat(newIndices, sameBeanAs(indices(0)));
    }

    @Test
    public void movesUpOnlyThoseActionsWhichAreNotOnTheTopAlready() {
        Actions actions = actionsWithElements(0, 1, 2, 3, 4, 5, 6);

        actions.moveUp(indices(0, 1, 4, 6));

        assertThat(actions, sameBeanAs(actionsWithElements(0, 1, 2, 4, 3, 6, 5)));
    }

    @Test
    public void doesNotLoseSelectionOfTheTopActionsWhenMovingMultipleActions() {
        Actions actions = actionsWithElements(0, 1, 2, 3, 4, 5, 6);

        int[] newIndices = actions.moveUp(indices(0, 1, 4, 6));

        assertThat(newIndices, sameBeanAs(indices(0, 1, 3, 5)));
    }

}