Imagine you have a method, which purpose is to modify an array of objects, and return the the new version of it.
const likePosts = (posts) => posts.map((post) => {
post.isLiked = true
return post;
});
const originalPosts = [{ isLiked: false }, { isLiked: false }];
const newLikedPosts = likePosts(originalPosts);
console.log(newLikedPosts);But, as JS non-primitive values are mutable, if after executing the code, we log the originalPosts, we will see how the isLiked field is not false, it has been modified to true too!
const likePosts = (posts) => posts.map((post) => {
post.isLiked = true
return post;
});
const originalPosts = [{ isLiked: false }, { isLiked: false }];
const newLikedPosts = likePosts(originalPosts);
console.log({ newLikedPosts });
console.log({ originalPosts });Should we care about this thing? I mean, for example, in this scenario, it does not matter whether "originalPosts" is updated, since it is useless data for the code after executing "likePosts".
If we care about data mutation when it is not necessary, we might be doing extra cloning and, consequently, negatively affecting performance, right?
In order words, when you code JS, are you constantly cloning data to avoid mutations where is it not really necessary?
CodePudding user response：
Yes, it's something worth thinking and caring about. In scripts, at least in my experience and from what I've seen on Stack Overflow, quite a lot of bugs are caused by a mutable value changing when the script-writer wasn't expecting it to.
With your code:
const likePosts = (posts) => posts.map((post) => {
If someone else maintaining this script (or you, a few months down the line) sees this function, it'd be entirely reasonable for them to assume that it creates a new array, instead of modifying the objects in the existing one - that's what .map is for, and that's what most people will expect it to do. If, however, you mutate the object inside the callback, and then return the original object, there's not much point to using .map - and those who have to maintain the code later could easily be mislead by the intent of the code, which could lead to bugs.
Sure, for such a small function, such a thing is unlikely, but for the general case, things are rarely so simple. IMO, returning new objects instead of mutating existing ones often leads to cleaner, more understandable code - there's a huge programming paradigm built on that and similar ideas - but you're right that:
we might be doing extra cloning and, consequently, negatively affecting performance, right?
Creating entirely new objects when not necessary could affect performance, especially if you need to do so frequently and the objects that would otherwise have to be cloned are large and/or deep. But, computers nowadays are pretty good - it's pretty unlikely that such a difference (mutating vs creating new objects) would have any discernable effect in a given section of code, even on a low-end machine. It might, but it probably won't.
As always, if performance is something you're worried about, first make sure you have working, clear, and maintainable code - and then you can run performance tests to identify the bottleneck(s), and fix the bottlenecks. Premature optimization takes time, can make code harder to understand, and may not have any effect on the resulting app as a whole.
In most cases, when you have to choose between mutation and creating new objects, you can probably choose either, and your app will be just as good regardless, as long as the intent of the code is clear. For example, you might change
const likePosts = (posts) => posts.map((post) => {
post.isLiked = true
return post;
});
to
const likePosts = (posts) => {
for (const post of posts) {
post.isLiked = true;
}
};
The lack of a return value, and the lack of a superfluous .map, makes the intent much clearer.