8000 refactor: Refactor the equals function for readability by TehShrike · Pull Request #13650 · angular/angular.js · GitHub
[go: up one dir, main page]
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

refactor: Refactor the equals function for readability #13650

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
refactor: Refactor the equals function for readability 
- Turn an unnecessary nested if block into an &&
- Change the isRegExp block to use the same "if the second argument is the wrong type, return
false" pattern as all the other blocks
  • Loading branch information
@TehShrike
TehShrike committed Dec 29, 2015
commit b29a1ed76204e3121e1cbe7dde458293738b2785
55 changes: 27 additions & 28 deletions src/Angular.js
View file
Open in desktop
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
0