Review and alternatives.
- Use strict mode. This should go without saying, however I add it as a point in reviews when I see bugs or potencial bugs that would be caught easily.
- Never use a variable that has not been declared. An example in your code is
encode and if you were in strict mode you would have found it before you posted the review.
- Use constants when the variable does not change. In your example you declare
result as a variable, and define it as an array. But you never change the variable. It should be declared as a constant. const result = [];
- Comments are bad. They clutter up the source and are seldom of any help. Only coders will read your code and stating the obvious helps no one, and creating ambiguity via comments is dangerous. See more on comments below
- Don't clutter your source with redundant code, less is best.
- Don't repeat. You have the variable
len and the only time you use it you subtract 1 from it, repeating computations for no reason. Better to subtract the one when you define the value const lastIdx = array.length;
- When iterating use
for of loops in favour of array methods as they are slightly quicker, can be exited early unlike many array methods, and tend to be more suited to the logic and need of the code.
Don't use JSON to compare.
- JSON.stringify is slow and should not be used unless you are saving or transporting data.
- JSON.stringify will throw on cyclic references, meaning your compare function can throw even though the code state is good.
- JSON.stringify converts undefined array
items to null, meaning that is you compare [undefined] and [null] they will be equal.
- JSON.stringify can be customised and thus there is no guarantee that it accurately reflects the data content.
Run length encoding is about the equality of items. A more detailed comparison will make for a better encoder. See rewrites for two examples.
Redundent code
You had
arrayEquals = (...arr) => {
return arr[0].map(x => {
return JSON.stringify(x);
}).every((x, _, a) => {
return x === a[0];
});
};
The returns are not needed
const arrayEquals = (...arr) => arr[0].map(x => JSON.stringify(x)).every((x, _, a) => x === a[0]);
or
const arrayEquals = (...arr) => {
return arr[0]
.map(x => JSON.stringify(x))
.every((x, _, a) => x === a[0]);
};
or
const arrayEquals = (...arr) => {
return arr[0].map(x => JSON.stringify(x)).every((x, _, a) => x === a[0]);
};
Comments
Comments are rare and only when the immediate code (within eye shoot) is lacking needed abstract context, or has side effects beyond the scope of the current context.
Comment are not checked or vetted, and can remain in source even when the code that is commented has changed.
This mean it is easy for comments to lie, and an old comment can make your intent ambiguous. This is very dangerous when you later make changes, what do you believe the comment or the code.
If you feel some code needs a comment, before adding a comment ask yourself "Can I change the naming to make the code more readable?" Most of the time the answer will be yes.
Example of a poor comment in your code
// if current value differs from last
if (i !== len - 1) {
The comment is confusing, i is an index, \\ current val infers a data value, and \\ last is obscure so anyone reading the comment will still have to find len to workout what it is anyway
You could have commented
if (i !== len - 1) { // is not last index
or better let the code say it all
if (i !== arrayLen - 1) {
Personally I use idx for callback indexes, rather than i and resurve i, j, k for use in loops only.
Example of stating the obvious in your code
count = 0; // reset the count
Does that code really need clarification?
Rewrites
The following rewrites simplify the encoding, while making the comparison more robust.
There are two versions. The first is cyclic unsafe and will throw a range error if the array contains references to itself. The second uses a depth limit to stop endless recursion marking such items as equal.
Cyclic unsafe version
const runLengthEncode = array => {
"use strict";
const result = [];
var item, i = 0;
const isArr = arr => Array.isArray(arr);
const compare = (item1, item2) => {
if (isArr(item1) && isArr(item2)) {
if (item1.length === item2.length) {
for (let i = 0; i < item1.length; i++) {
if (!compare(item1[i], item2[i])) { return false }
}
} else { return false }
}else if(item1 !== item2) { return false }
return true;
}
while (i < array.length) {
if (item && compare(item[1], array[i])) { item[0] += 1 }
else { result.push(item = [1, array[i]]) }
i += 1;
}
return result;
}
// From here down is support code not part of answer
const array2Str = arr => "[" + arr.map(d => Array.isArray(d) ? array2Str(d) : d) + "]";
const log= data => { const div = document.createElement("div"); div.textContent = data; logEl.appendChild(div)}
const encode = d => {
log("-------------------");
log("Data " + array2Str(d));
log("encode as " + array2Str(runLengthEncode(d)));
}
log(encode ([1, 1, 2, 2, 3, 3, 3, 3, 4]));
log(encode ([4, 4, 4, 4, 4]));
log(encode ([[1, 2, 3], [4, 5, 6], [7, 8, 9], [10, 11, 12]]));
log(encode ([[1, 2, 3], [1, 2, 3], [4, 5, 6]]));
log(encode ([[0], [0], [1], [2, 3], [4, 5, 6], [], []]));
<code id="logEl"></code>
Example 2 cyclic safe.
"use strict";
const runLengthEncode = (array, maxDepth = 4) => {
const result = [];
var item, i = 0, depth = 0;
const isArr = arr => Array.isArray(arr);
const compare = (item1, item2) => {
var res = true;
if (depth++ < maxDepth) {
if (isArr(item1) && isArr(item2)) {
if (item1.length === item2.length) {
for (let i = 0; i < item1.length && res; i++) {
if (!compare(item1[i], item2[i])) { res = false }
}
} else { res = false }
}else if(item1 !== item2) { res = false }
}
depth --;
return res;
}
const compareItems = (item1, item2) => (depth = 0, compare(item1, item2));
while(i < array.length){
if (item && compareItems(item[1], array[i])) { item[0] += 1 }
else { result.push(item = [1, array[i]]) }
i += 1;
}
return result;
}
// From here down is support code not part of answer
const array2Str = arr => "[" + arr.map(d => Array.isArray(d) ? array2Str(d) : d) + "]";
const log= data => { const div = document.createElement("div"); div.textContent = data; logEl.appendChild(div)}
const encode = d => {
log("-------------------");
log("Data " + array2Str(d));
log("encode as " + array2Str(runLengthEncode(d)));
}
const encodeC = d => {
log("--Cyclic test (arrays not formated and Cyclic arrays return empty strings) ----------");
log("Data " + d);
log("encode as " + runLengthEncode(d));
}
log("Test cyclic array");
var a = [1,a];
a.push(a);
log(encodeC ([a, a, a]));
log("-------------------");
log("Standard tests");
log(encode ([1, 1, 2, 2, 3, 3, 3, 3, 4]));
log(encode ([4, 4, 4, 4, 4]));
log(encode ([[1, 2, 3], [4, 5, 6], [7, 8, 9], [10, 11, 12]]));
log(encode ([[1, 2, 3], [1, 2, 3], [4, 5, 6]]));
log(encode ([[0], [0], [1], [2, 3], [4, 5, 6], [], []]));
<code id="logEl"></code>
