Closed angelsolaorbaiceta closed 3 months ago
Hi @angelsolaorbaiceta
Thanks for the detailed explanation! Your code is an excellent example of practicing SOLID principles. I always learn a lot from your implementations and explanations.
This update perfectly solves the duplication problems. Since the sequence of items in the output arrays is not essential, I updated the testing method to only consider the count of items being added or removed:
it("array items being removed", () => {
const oldArr = [1, 1, 2, 2, 1, 4, 3, 4, 5, 1, 1, 5, 7, 8, 9];
const newArr = [1, 2, 3, 4];
const diff = arraysDiff(oldArr, newArr);
expect(makeCountMap(diff.added)).toEqual(new Map([]));
expect(makeCountMap(diff.removed)).toEqual(
new Map([
[1, 4],
[2, 1],
[4, 1],
[5, 2],
[7, 1],
[8, 1],
[9, 1],
]),
);
});
I have one additional thought regarding the for loop in the arraysDiff(oldArray, newArray)
function:
for (const key of diff.updated) {
const oldCount = oldsCount.get(key)
const newCount = newsCount.get(key)
const diff = newCount - oldCount
if (diff > 0) {
added.push(...Array(diff).fill(key))
} else {
removed.push(...Array(-diff).fill(key))
}
}
The variable name diff
is similar to the object diff
we’re looping over. While it won’t break the program or tests, it could potentially cause some confusion. Do you think we could use a name like diffCount
to avoid such confusion?
Hi @angelsolaorbaiceta
Thanks for the detailed explanation! Your code is an excellent example of practicing SOLID principles. I always learn a lot from your implementations and explanations.
This update perfectly solves the duplication problems. Since the sequence of items in the output arrays is not essential, I updated the testing method to only consider the count of items being added or removed:
it("array items being removed", () => { const oldArr = [1, 1, 2, 2, 1, 4, 3, 4, 5, 1, 1, 5, 7, 8, 9]; const newArr = [1, 2, 3, 4]; const diff = arraysDiff(oldArr, newArr); expect(makeCountMap(diff.added)).toEqual(new Map([])); expect(makeCountMap(diff.removed)).toEqual( new Map([ [1, 4], [2, 1], [4, 1], [5, 2], [7, 1], [8, 1], [9, 1], ]), ); });
I have one additional thought regarding the for loop in the
arraysDiff(oldArray, newArray)
function:for (const key of diff.updated) { const oldCount = oldsCount.get(key) const newCount = newsCount.get(key) const diff = newCount - oldCount if (diff > 0) { added.push(...Array(diff).fill(key)) } else { removed.push(...Array(-diff).fill(key)) } }
The variable name
diff
is similar to the objectdiff
we’re looping over. While it won’t break the program or tests, it could potentially cause some confusion. Do you think we could use a name likediffCount
to avoid such confusion?
You are very right! I'll rename that variable to make it clearer. Thanks for the feedback.
arraysDiff()
handles duplicate entriesCloses #260 .
As noted by @terrence-ou , the current implementation of the
arraysDiff()
function doesn't deal with duplicate values. For example:Would incorrectly return:
When the correct solution would have been that two
1
s were added:This error didn't surface earlier because, the only use of the
arraysDiff()
function is to find the CSS classes that were added/removed from an element. The browser deduplicates the CSS classes added to an element, and thus, this error has no visible effect in the inner working of the framework. It's nevertheless a very interesting problem.The Problem
The problem is in how the added and removed items are searched for:
newArray.filter((newItem) => !oldArray.includes(newItem))
oldArray.filter((oldItem) => !newArray.includes(oldItem))
That
includes()
usage stops at the first match, thus missing duplicates.Solution
Solving this problem requires to count how many times a given item appears in each array. A count map is created for each array for this purpose:
The
makeCountMap()
function is straightforward to implement:Then, we diff the keys of both maps:
The
mapsDiff()
function implementation is equivalent to theobjectsDiff()
one, but this time, the keys can be anything and not juststring
s orSymbol
s:Then, in the
arraysDiff()
function, we have not only to see what items were added or removed, but count how many times they were added or removed:And lastly, for the keys that were updated (items that appear in both arrays, but in a different number) we want to calculate the difference between how many times it appears in the new versus the old array. Based on the result, we include it as addition or removal:
And hence the complete implementation is:
Thanks @terrence-ou for reporting this error! 🤝