Style
Your function name doesn't tell me whether you compute the symmetric or non-symmetric set difference. How about symmetricDifference
instead of diffArray
?
Instead of newArr
I'd prefer reading e.g. result
which tells me something about the role of that variable instead of its type.
There is an empty else { }
clause in the first inner loop but not the second inner loop. Without that purely stylistic difference, the similarity of these two code blocks becomes more apparent.
The statements within the succeeding if
clauses are identical, so you can merge them into one by combining the conditions with a logical OR operator.
The type checks and equality tests performed within the if
conditions seem overly complex. AFAIK ==
is identical to ===
when both operands have the same type, so there is no need to mix them. Also, a generic function named diffArray
should handle all possible array values, not only numbers and strings. This would allow you to simplify the two if
conditions to e.g. a simple if (arr2[i] === arr1[j]) { ... }
.
The iterator variable i
is used to iterate through arr2
while iterator variable j
is used to iterate through arr1
. Readability improves by matching i
with arr1
and j
with arr2
according to their alphabetical order.
Performance
The inner loops are counting how often elements from the first array appear in the second array and vice versa. However, you are not interested in the exact count, only if count > 0
. So you could use a labeled continue
as soon as the count increments for the very first time:
outer: for (var i = 0; i < arr1.length; i++) {
for (var j = 0; j < arr2.length; j++) {
if (arr1[i] === arr2[j]) continue outer;
}
result.push(arr1[i]);
}
Now, it is much easier to see that the inner loop is actually just checking the existence of an element within an array. You can use the faster built-in indexOf
method instead:
for (var i = 0; i < arr1.length; i++) {
if (arr2.indexOf(arr1[i]) < 0) result.push(arr1[i]);
}
Even better, use the newer and more explicit includes
method. However, this changes the semantics of your code due to the different handling of NaN
:
NaN === NaN // false
[NaN].indexOf(NaN) // -1
[NaN].includes(NaN) // true
Now, your updated code could look as follows:
function symmetricDifference(arr1, arr2) {
const result = [];
for (const val of arr1) {
if (!arr2.includes(val)) result.push(val);
}
for (const val of arr2) {
if (!arr1.includes(val)) result.push(val);
}
return result;
}
Declarative vs. Imperative
Those two remaining loops actually filter the input arrays and return the remaining unique values. A more descriptive and possibly self-documenting way of writing such an operation is given by the filter
method:
function symmetricDifference(arr1, arr2) {
const difference1 = arr1.filter(val => !arr2.includes(val));
const difference2 = arr2.filter(val => !arr1.includes(val));
return difference1.concat(difference2);
}
However, the performance of plain for
loops is superior.
Runtime Complexity
If you have to deal with larger arrays and prefer to have an implementation with higher setup costs but linear instead of quadratic runtime complexity, convert the input arrays into sets first and use the much faster set.has(val)
instead of arr.includes(val)
.
Generalization
A generic solution which is not restricted to arrays but handles any iterable input could look as follows:
function* symmetricDifference(iterable1, iterable2) {
const set1 = new Set(iterable1);
const set2 = new Set(iterable2);
for (const val of set1) if (!set2.has(val)) yield val;
for (const val of set2) if (!set1.has(val)) yield val;
}