Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

refactor: Refactor the equals function for readability #13650

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 27 additions & 28 deletions src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -969,38 +969,37 @@ function equals(o1, o2) {
if (o1 === null || o2 === null) return false;
if (o1 !== o1 && o2 !== o2) return true; // NaN === NaN
var t1 = typeof o1, t2 = typeof o2, length, key, keySet;
if (t1 == t2) {
if (t1 == 'object') {
if (isArray(o1)) {
if (!isArray(o2)) return false;
if ((length = o1.length) == o2.length) {
for (key = 0; key < length; key++) {
if (!equals(o1[key], o2[key])) return false;
}
return true;
}
} else if (isDate(o1)) {
if (!isDate(o2)) return false;
return equals(o1.getTime(), o2.getTime());
} else if (isRegExp(o1)) {
return isRegExp(o2) ? o1.toString() == o2.toString() : false;
} else {
if (isScope(o1) || isScope(o2) || isWindow(o1) || isWindow(o2) ||
isArray(o2) || isDate(o2) || isRegExp(o2)) return false;
keySet = createMap();
for (key in o1) {
if (key.charAt(0) === '$' || isFunction(o1[key])) continue;
if (t1 == t2 && t1 == 'object') {
if (isArray(o1)) {
if (!isArray(o2)) return false;
if ((length = o1.length) == o2.length) {
for (key = 0; key < length; key++) {
if (!equals(o1[key], o2[key])) return false;
keySet[key] = true;
}
for (key in o2) {
if (!(key in keySet) &&
key.charAt(0) !== '$' &&
isDefined(o2[key]) &&
!isFunction(o2[key])) return false;
}
return true;
}
} else if (isDate(o1)) {
if (!isDate(o2)) return false;
return equals(o1.getTime(), o2.getTime());
} else if (isRegExp(o1)) {
if (!isRegExp(o2)) return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] I find return isRegExp(o2) && o1.toString() == o2.toString(); easier to read, but not a blocker

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me too, but every other if (some type) block used the other pattern, and the function as a whole seemed easier to read when they all followed the same pattern.

return o1.toString() == o2.toString();
} else {
if (isScope(o1) || isScope(o2) || isWindow(o1) || isWindow(o2) ||
isArray(o2) || isDate(o2) || isRegExp(o2)) return false;
keySet = createMap();
for (key in o1) {
if (key.charAt(0) === '$' || isFunction(o1[key])) continue;
if (!equals(o1[key], o2[key])) return false;
keySet[key] = true;
}
for (key in o2) {
if (!(key in keySet) &&
key.charAt(0) !== '$' &&
isDefined(o2[key]) &&
!isFunction(o2[key])) return false;
}
return true;
}
}
return false;
Expand Down