Skip to content

Commit

Permalink
# Bug Fixes
Browse files Browse the repository at this point in the history
Submitting our solution revealed a few bugs in our solution. The first
bug can be pointed out with this failing test:

    expect([].sameStructureAs(1)).to.be.false;

We were assuming that `other` would be an array. Unfortunately in this
case, `other` is `1`, which causes `_.zipWith` to return an empty array.

We can fix this bug by adding an upfront check asserting that `other` is
an `Array`.

After fixing that issue, another bug reared its ugly head:

    expect([1].sameStructureAs([1, 2])).to.be.false;

In the last iteration of `_.zipWith`, `a` was `undefined` and `b` was
`2`. While checking if both of these values were not arrays returned
true, we didn't check if both values existed.

The fix for this bug is to check that both `a` and `b` are not
`undefined`:

    let bothDefined = !_.isUndefined(a) && !_.isUndefined(b);
    let bothNot = bothDefined && !_.isArray(a) && !_.isArray(b);

With those fixes, out suite flips back to green!
  • Loading branch information
pcorey committed Aug 1, 2016
1 parent 4046fc7 commit 6a37908
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
6 changes: 5 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import _ from "lodash";

Array.prototype.sameStructureAs = function(other) {
if (!_.isArray(other)) {
return false;
}
let comparisons = _.zipWith(this, other, (a, b) => {
let bothArrays = _.isArray(a) && _.isArray(b);
let bothNot = !_.isArray(a) && !_.isArray(b);
let bothDefined = !_.isUndefined(a) && !_.isUndefined(b);
let bothNot = bothDefined && !_.isArray(a) && !_.isArray(b);
return (bothArrays && a.sameStructureAs(b)) || bothNot;
});
return _.every(comparisons);
Expand Down
2 changes: 2 additions & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ describe("sameStructureAs", function() {
expect([[], 1].sameStructureAs([[], []])).to.be.false;
expect([[], [1]].sameStructureAs([[], [1]])).to.be.true;
expect([[], [1]].sameStructureAs([[], [[]]])).to.be.false;
expect([].sameStructureAs(1)).to.be.false;
expect([1].sameStructureAs([1, 2])).to.be.false;
});

});

0 comments on commit 6a37908

Please sign in to comment.