-
Notifications
You must be signed in to change notification settings - Fork 984
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ArrayMove mutates array #61
Comments
Completely agree. However, the other solutions I looked at were incredibly expensive. Happy to review any PR you may have that would solve this 👍 Thankfully |
Why not just 'copy' the array? |
Here are three possible implementations of an immutable function arrayMoveCopy(array, previousIndex, newIndex) {
return arrayMove(array.slice(), previousIndex, newIndex);
}
function arrayMoveImmutable(array, previousIndex, newIndex) {
var length = array.length;
var newLength = newIndex >= length ? newIndex + 1 : length;
var newArray = new Array(newLength);
var element = array[previousIndex];
for (var i = 0, j = 0; i < newLength; i++) {
if (i === newIndex) {
newArray[i] = element;
} else {
if (j === previousIndex) {
j++;
}
if (j < length) {
newArray[i] = array[j++];
} else {
newArray[i] = undefined;
}
}
}
return newArray;
}
function arrayMoveImmutableReverse(array, previousIndex, newIndex) {
var length = array.length;
var newLength = newIndex >= length ? newIndex + 1 : length;
var newArray = new Array(newLength);
var element = array[previousIndex];
var i = newLength;
var j = newLength;
while (i--) {
if (i === newIndex) {
newArray[i] = element;
} else {
j--;
if (j === previousIndex) {
j--;
}
if (j < length) {
newArray[i] = array[j];
} else {
newArray[i] = undefined;
}
}
}
return newArray;
} |
@zaygraveyard my solution was based on slicing and concating array parts, but your solution with one level loop should have better performance |
I've created a simple solution for this issue here, but i guess we better measure performance for every example above, so we can understand which implementation is the fastest. |
Thanks for your PR @canvaskisa, it's been merged back into master. Regarding performance concerns: I think if performance is a real concern for your application (for instance, if you have an array with millions of records), you're free to opt out and use your own implementation, as Thanks for your input everyone |
We should avoid object mutations
https://github.com/clauderic/react-sortable-hoc/blob/master/src/utils.js#L1-L10
The text was updated successfully, but these errors were encountered: