refactor(Differ): cleanup

This commit is contained in:
Victor Berchet 2016-07-15 16:26:19 -07:00
parent b6746cce9c
commit 0914dc35e8
5 changed files with 153 additions and 209 deletions

View File

@ -25,10 +25,9 @@ export class ViewAnimationMap {
} }
findAllPlayersByElement(element: any): AnimationPlayer[] { findAllPlayersByElement(element: any): AnimationPlayer[] {
var players: AnimationPlayer[] = []; const el = this._map.get(element);
StringMapWrapper.forEach(
this._map.get(element), (player: AnimationPlayer) => players.push(player)); return el ? StringMapWrapper.values(el) : [];
return players;
} }
set(element: any, animationName: string, player: AnimationPlayer): void { set(element: any, animationName: string, player: AnimationPlayer): void {

View File

@ -6,15 +6,14 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {MapWrapper, StringMapWrapper} from '../../facade/collection'; import {StringMapWrapper} from '../../facade/collection';
import {BaseException} from '../../facade/exceptions'; import {BaseException} from '../../facade/exceptions';
import {isBlank, isJsObject, looseIdentical, stringify} from '../../facade/lang'; import {isJsObject, looseIdentical, stringify} from '../../facade/lang';
import {ChangeDetectorRef} from '../change_detector_ref'; import {ChangeDetectorRef} from '../change_detector_ref';
import {KeyValueDiffer, KeyValueDifferFactory} from './keyvalue_differs'; import {KeyValueDiffer, KeyValueDifferFactory} from './keyvalue_differs';
/* @ts2dart_const */
export class DefaultKeyValueDifferFactory implements KeyValueDifferFactory { export class DefaultKeyValueDifferFactory implements KeyValueDifferFactory {
constructor() {} constructor() {}
supports(obj: any): boolean { return obj instanceof Map || isJsObject(obj); } supports(obj: any): boolean { return obj instanceof Map || isJsObject(obj); }
@ -38,67 +37,64 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer {
this._removalsHead !== null; this._removalsHead !== null;
} }
forEachItem(fn: Function) { forEachItem(fn: (r: KeyValueChangeRecord) => void) {
var record: KeyValueChangeRecord; var record: KeyValueChangeRecord;
for (record = this._mapHead; record !== null; record = record._next) { for (record = this._mapHead; record !== null; record = record._next) {
fn(record); fn(record);
} }
} }
forEachPreviousItem(fn: Function) { forEachPreviousItem(fn: (r: KeyValueChangeRecord) => void) {
var record: KeyValueChangeRecord; var record: KeyValueChangeRecord;
for (record = this._previousMapHead; record !== null; record = record._nextPrevious) { for (record = this._previousMapHead; record !== null; record = record._nextPrevious) {
fn(record); fn(record);
} }
} }
forEachChangedItem(fn: Function) { forEachChangedItem(fn: (r: KeyValueChangeRecord) => void) {
var record: KeyValueChangeRecord; var record: KeyValueChangeRecord;
for (record = this._changesHead; record !== null; record = record._nextChanged) { for (record = this._changesHead; record !== null; record = record._nextChanged) {
fn(record); fn(record);
} }
} }
forEachAddedItem(fn: Function) { forEachAddedItem(fn: (r: KeyValueChangeRecord) => void) {
var record: KeyValueChangeRecord; var record: KeyValueChangeRecord;
for (record = this._additionsHead; record !== null; record = record._nextAdded) { for (record = this._additionsHead; record !== null; record = record._nextAdded) {
fn(record); fn(record);
} }
} }
forEachRemovedItem(fn: Function) { forEachRemovedItem(fn: (r: KeyValueChangeRecord) => void) {
var record: KeyValueChangeRecord; var record: KeyValueChangeRecord;
for (record = this._removalsHead; record !== null; record = record._nextRemoved) { for (record = this._removalsHead; record !== null; record = record._nextRemoved) {
fn(record); fn(record);
} }
} }
diff(map: Map<any, any>): any { diff(map: Map<any, any>|{[k: string]: any}): any {
if (isBlank(map)) map = MapWrapper.createFromPairs([]); if (!map) {
if (!(map instanceof Map || isJsObject(map))) { map = new Map();
} else if (!(map instanceof Map || isJsObject(map))) {
throw new BaseException(`Error trying to diff '${map}'`); throw new BaseException(`Error trying to diff '${map}'`);
} }
if (this.check(map)) { return this.check(map) ? this : null
return this;
} else {
return null;
}
} }
onDestroy() {} onDestroy() {}
check(map: Map<any, any>): boolean { check(map: Map<any, any>|{[k: string]: any}): boolean {
this._reset(); this._reset();
var records = this._records; let records = this._records;
var oldSeqRecord: KeyValueChangeRecord = this._mapHead; let oldSeqRecord: KeyValueChangeRecord = this._mapHead;
var lastOldSeqRecord: KeyValueChangeRecord = null; let lastOldSeqRecord: KeyValueChangeRecord = null;
var lastNewSeqRecord: KeyValueChangeRecord = null; let lastNewSeqRecord: KeyValueChangeRecord = null;
var seqChanged: boolean = false; let seqChanged: boolean = false;
this._forEach(map, (value: any /** TODO #9100 */, key: any /** TODO #9100 */) => { this._forEach(map, (value: any, key: any) => {
var newSeqRecord: any /** TODO #9100 */; let newSeqRecord: any;
if (oldSeqRecord !== null && key === oldSeqRecord.key) { if (oldSeqRecord && key === oldSeqRecord.key) {
newSeqRecord = oldSeqRecord; newSeqRecord = oldSeqRecord;
if (!looseIdentical(value, oldSeqRecord.currentValue)) { if (!looseIdentical(value, oldSeqRecord.currentValue)) {
oldSeqRecord.previousValue = oldSeqRecord.currentValue; oldSeqRecord.previousValue = oldSeqRecord.currentValue;
@ -108,7 +104,6 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer {
} else { } else {
seqChanged = true; seqChanged = true;
if (oldSeqRecord !== null) { if (oldSeqRecord !== null) {
oldSeqRecord._next = null;
this._removeFromSeq(lastOldSeqRecord, oldSeqRecord); this._removeFromSeq(lastOldSeqRecord, oldSeqRecord);
this._addToRemovals(oldSeqRecord); this._addToRemovals(oldSeqRecord);
} }
@ -134,7 +129,7 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer {
} }
lastOldSeqRecord = oldSeqRecord; lastOldSeqRecord = oldSeqRecord;
lastNewSeqRecord = newSeqRecord; lastNewSeqRecord = newSeqRecord;
oldSeqRecord = oldSeqRecord === null ? null : oldSeqRecord._next; oldSeqRecord = oldSeqRecord && oldSeqRecord._next;
}); });
this._truncate(lastOldSeqRecord, oldSeqRecord); this._truncate(lastOldSeqRecord, oldSeqRecord);
return this.isDirty; return this.isDirty;
@ -143,7 +138,7 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer {
/** @internal */ /** @internal */
_reset() { _reset() {
if (this.isDirty) { if (this.isDirty) {
var record: KeyValueChangeRecord; let record: KeyValueChangeRecord;
// Record the state of the mapping // Record the state of the mapping
for (record = this._previousMapHead = this._mapHead; record !== null; record = record._next) { for (record = this._previousMapHead = this._mapHead; record !== null; record = record._next) {
record._nextPrevious = record._next; record._nextPrevious = record._next;
@ -157,31 +152,6 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer {
record.previousValue = record.currentValue; record.previousValue = record.currentValue;
} }
// todo(vicb) once assert is supported
// assert(() {
// var r = _changesHead;
// while (r != null) {
// var nextRecord = r._nextChanged;
// r._nextChanged = null;
// r = nextRecord;
// }
//
// r = _additionsHead;
// while (r != null) {
// var nextRecord = r._nextAdded;
// r._nextAdded = null;
// r = nextRecord;
// }
//
// r = _removalsHead;
// while (r != null) {
// var nextRecord = r._nextRemoved;
// r._nextRemoved = null;
// r = nextRecord;
// }
//
// return true;
//});
this._changesHead = this._changesTail = null; this._changesHead = this._changesTail = null;
this._additionsHead = this._additionsTail = null; this._additionsHead = this._additionsTail = null;
this._removalsHead = this._removalsTail = null; this._removalsHead = this._removalsTail = null;
@ -197,17 +167,12 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer {
lastRecord._next = null; lastRecord._next = null;
} }
var nextRecord = record._next; var nextRecord = record._next;
// todo(vicb) assert
// assert((() {
// record._next = null;
// return true;
//}));
this._addToRemovals(record); this._addToRemovals(record);
lastRecord = record; lastRecord = record;
record = nextRecord; record = nextRecord;
} }
for (var rec: KeyValueChangeRecord = this._removalsHead; rec !== null; rec = rec._nextRemoved) { for (let rec: KeyValueChangeRecord = this._removalsHead; rec !== null; rec = rec._nextRemoved) {
rec.previousValue = rec.currentValue; rec.previousValue = rec.currentValue;
rec.currentValue = null; rec.currentValue = null;
this._records.delete(rec.key); this._records.delete(rec.key);
@ -222,12 +187,6 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer {
/** @internal */ /** @internal */
_addToRemovals(record: KeyValueChangeRecord) { _addToRemovals(record: KeyValueChangeRecord) {
// todo(vicb) assert
// assert(record._next == null);
// assert(record._nextAdded == null);
// assert(record._nextChanged == null);
// assert(record._nextRemoved == null);
// assert(record._prevRemoved == null);
if (this._removalsHead === null) { if (this._removalsHead === null) {
this._removalsHead = this._removalsTail = record; this._removalsHead = this._removalsTail = record;
} else { } else {
@ -245,22 +204,13 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer {
} else { } else {
prev._next = next; prev._next = next;
} }
// todo(vicb) assert record._next = null;
// assert((() {
// record._next = null;
// return true;
//})());
} }
/** @internal */ /** @internal */
_removeFromRemovals(record: KeyValueChangeRecord) { _removeFromRemovals(record: KeyValueChangeRecord) {
// todo(vicb) assert const prev = record._prevRemoved;
// assert(record._next == null); const next = record._nextRemoved;
// assert(record._nextAdded == null);
// assert(record._nextChanged == null);
var prev = record._prevRemoved;
var next = record._nextRemoved;
if (prev === null) { if (prev === null) {
this._removalsHead = next; this._removalsHead = next;
} else { } else {
@ -276,12 +226,6 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer {
/** @internal */ /** @internal */
_addToAdditions(record: KeyValueChangeRecord) { _addToAdditions(record: KeyValueChangeRecord) {
// todo(vicb): assert
// assert(record._next == null);
// assert(record._nextAdded == null);
// assert(record._nextChanged == null);
// assert(record._nextRemoved == null);
// assert(record._prevRemoved == null);
if (this._additionsHead === null) { if (this._additionsHead === null) {
this._additionsHead = this._additionsTail = record; this._additionsHead = this._additionsTail = record;
} else { } else {
@ -292,11 +236,6 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer {
/** @internal */ /** @internal */
_addToChanges(record: KeyValueChangeRecord) { _addToChanges(record: KeyValueChangeRecord) {
// todo(vicb) assert
// assert(record._nextAdded == null);
// assert(record._nextChanged == null);
// assert(record._nextRemoved == null);
// assert(record._prevRemoved == null);
if (this._changesHead === null) { if (this._changesHead === null) {
this._changesHead = this._changesTail = record; this._changesHead = this._changesTail = record;
} else { } else {
@ -306,12 +245,12 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer {
} }
toString(): string { toString(): string {
var items: any[] /** TODO #9100 */ = []; const items: any[] = [];
var previous: any[] /** TODO #9100 */ = []; const previous: any[] = [];
var changes: any[] /** TODO #9100 */ = []; const changes: any[] = [];
var additions: any[] /** TODO #9100 */ = []; const additions: any[] = [];
var removals: any[] /** TODO #9100 */ = []; const removals: any[] = [];
var record: KeyValueChangeRecord; let record: KeyValueChangeRecord;
for (record = this._mapHead; record !== null; record = record._next) { for (record = this._mapHead; record !== null; record = record._next) {
items.push(stringify(record)); items.push(stringify(record));
@ -337,9 +276,9 @@ export class DefaultKeyValueDiffer implements KeyValueDiffer {
} }
/** @internal */ /** @internal */
_forEach(obj: any /** TODO #9100 */, fn: Function) { private _forEach<K, V>(obj: Map<K, V>|{[k: string]: V}, fn: (v: V, k: any) => void) {
if (obj instanceof Map) { if (obj instanceof Map) {
(<Map<any, any>>obj).forEach(<any>fn); obj.forEach(fn);
} else { } else {
StringMapWrapper.forEach(obj, fn); StringMapWrapper.forEach(obj, fn);
} }

View File

@ -8,16 +8,13 @@
import {DefaultKeyValueDiffer, DefaultKeyValueDifferFactory} from '@angular/core/src/change_detection/differs/default_keyvalue_differ'; import {DefaultKeyValueDiffer, DefaultKeyValueDifferFactory} from '@angular/core/src/change_detection/differs/default_keyvalue_differ';
import {afterEach, beforeEach, ddescribe, describe, expect, iit, it, xit} from '@angular/core/testing/testing_internal'; import {afterEach, beforeEach, ddescribe, describe, expect, iit, it, xit} from '@angular/core/testing/testing_internal';
import {NumberWrapper, isJsObject} from '../../../src/facade/lang';
import {kvChangesAsString} from '../../change_detection/util'; import {kvChangesAsString} from '../../change_detection/util';
// todo(vicb): Update the code & tests for object equality // todo(vicb): Update the code & tests for object equality
export function main() { export function main() {
describe('keyvalue differ', function() { describe('keyvalue differ', function() {
describe('DefaultKeyValueDiffer', function() { describe('DefaultKeyValueDiffer', function() {
var differ: any /** TODO #9100 */; var differ: DefaultKeyValueDiffer;
var m: Map<any, any>; var m: Map<any, any>;
beforeEach(() => { beforeEach(() => {
@ -113,108 +110,123 @@ export function main() {
})); }));
}); });
it('should test string by value rather than by reference (DART)', () => { it('should not see a NaN value as a change', () => {
m.set('foo', 'bar'); m.set('foo', Number.NaN);
differ.check(m);
var f = 'f';
var oo = 'oo';
var b = 'b';
var ar = 'ar';
m.set(f + oo, b + ar);
differ.check(m);
expect(differ.toString()).toEqual(kvChangesAsString({map: ['foo'], previous: ['foo']}));
});
it('should not see a NaN value as a change (JS)', () => {
m.set('foo', NumberWrapper.NaN);
differ.check(m); differ.check(m);
differ.check(m); differ.check(m);
expect(differ.toString()).toEqual(kvChangesAsString({map: ['foo'], previous: ['foo']})); expect(differ.toString()).toEqual(kvChangesAsString({map: ['foo'], previous: ['foo']}));
}); });
// JS specific tests (JS Objects) it('should work regardless key order', () => {
if (isJsObject({})) { m.set('a', 0);
describe('JsObject changes', () => { m.set('b', 0);
it('should support JS Object', () => { differ.check(m);
var f = new DefaultKeyValueDifferFactory();
expect(f.supports({})).toBeTruthy();
expect(f.supports('not supported')).toBeFalsy();
expect(f.supports(0)).toBeFalsy();
expect(f.supports(null)).toBeFalsy();
});
it('should do basic object watching', () => { m = new Map();
let m = {}; m.set('b', 1);
differ.check(m); m.set('a', 1);
differ.check(m);
(m as any /** TODO #9100 */)['a'] = 'A'; expect(differ.toString()).toEqual(kvChangesAsString({
differ.check(m); map: ['b[0->1]', 'a[0->1]'],
expect(differ.toString()) previous: ['a[0->1]', 'b[0->1]'],
.toEqual(kvChangesAsString({map: ['a[null->A]'], additions: ['a[null->A]']})); changes: ['b[0->1]', 'a[0->1]']
}));
});
(m as any /** TODO #9100 */)['b'] = 'B'; describe('JsObject changes', () => {
differ.check(m); it('should support JS Object', () => {
expect(differ.toString()) var f = new DefaultKeyValueDifferFactory();
.toEqual(kvChangesAsString( expect(f.supports({})).toBeTruthy();
{map: ['a', 'b[null->B]'], previous: ['a'], additions: ['b[null->B]']})); expect(f.supports('not supported')).toBeFalsy();
expect(f.supports(0)).toBeFalsy();
(m as any /** TODO #9100 */)['b'] = 'BB'; expect(f.supports(null)).toBeFalsy();
(m as any /** TODO #9100 */)['d'] = 'D';
differ.check(m);
expect(differ.toString()).toEqual(kvChangesAsString({
map: ['a', 'b[B->BB]', 'd[null->D]'],
previous: ['a', 'b[B->BB]'],
additions: ['d[null->D]'],
changes: ['b[B->BB]']
}));
m = {};
(m as any /** TODO #9100 */)['a'] = 'A';
(m as any /** TODO #9100 */)['d'] = 'D';
differ.check(m);
expect(differ.toString()).toEqual(kvChangesAsString({
map: ['a', 'd'],
previous: ['a', 'b[BB->null]', 'd'],
removals: ['b[BB->null]']
}));
m = {};
differ.check(m);
expect(differ.toString()).toEqual(kvChangesAsString({
previous: ['a[A->null]', 'd[D->null]'],
removals: ['a[A->null]', 'd[D->null]']
}));
});
}); });
describe('diff', () => { it('should do basic object watching', () => {
it('should return self when there is a change', () => { let m: {[k: string]: string} = {};
m.set('a', 'A'); differ.check(m);
expect(differ.diff(m)).toBe(differ);
});
it('should return null when there is no change', () => { m['a'] = 'A';
m.set('a', 'A'); differ.check(m);
differ.diff(m); expect(differ.toString())
expect(differ.diff(m)).toEqual(null); .toEqual(kvChangesAsString({map: ['a[null->A]'], additions: ['a[null->A]']}));
});
it('should treat null as an empty list', () => { m['b'] = 'B';
m.set('a', 'A'); differ.check(m);
differ.diff(m); expect(differ.toString())
expect(differ.diff(null).toString()) .toEqual(kvChangesAsString(
.toEqual(kvChangesAsString({previous: ['a[A->null]'], removals: ['a[A->null]']})); {map: ['a', 'b[null->B]'], previous: ['a'], additions: ['b[null->B]']}));
});
m['b'] = 'BB';
m['d'] = 'D';
differ.check(m);
expect(differ.toString()).toEqual(kvChangesAsString({
map: ['a', 'b[B->BB]', 'd[null->D]'],
previous: ['a', 'b[B->BB]'],
additions: ['d[null->D]'],
changes: ['b[B->BB]']
}));
m = {};
m['a'] = 'A';
m['d'] = 'D';
differ.check(m);
expect(differ.toString()).toEqual(kvChangesAsString({
map: ['a', 'd'],
previous: ['a', 'b[BB->null]', 'd'],
removals: ['b[BB->null]']
}));
m = {};
differ.check(m);
expect(differ.toString()).toEqual(kvChangesAsString({
previous: ['a[A->null]', 'd[D->null]'],
removals: ['a[A->null]', 'd[D->null]']
}));
it('should throw when given an invalid collection', () => {
expect(() => differ.diff('invalid')).toThrowError('Error trying to diff \'invalid\'');
});
}); });
}
it('should work regardless key order', () => {
let m: {[k: string]: number} = {a: 0, b: 0};
differ.check(m);
m = {b: 1, a: 1};
differ.check(m);
expect(differ.toString()).toEqual(kvChangesAsString({
map: ['b[0->1]', 'a[0->1]'],
previous: ['a[0->1]', 'b[0->1]'],
changes: ['b[0->1]', 'a[0->1]']
}));
});
});
describe('diff', () => {
it('should return self when there is a change', () => {
m.set('a', 'A');
expect(differ.diff(m)).toBe(differ);
});
it('should return null when there is no change', () => {
m.set('a', 'A');
differ.diff(m);
expect(differ.diff(m)).toEqual(null);
});
it('should treat null as an empty list', () => {
m.set('a', 'A');
differ.diff(m);
expect(differ.diff(null).toString())
.toEqual(kvChangesAsString({previous: ['a[A->null]'], removals: ['a[A->null]']}));
});
it('should throw when given an invalid collection', () => {
expect(() => differ.diff(<any>'invalid'))
.toThrowError('Error trying to diff \'invalid\'');
});
});
}); });
}); });
} }

View File

@ -116,12 +116,10 @@ export class StringMapWrapper {
return map.hasOwnProperty(key) ? map[key] : undefined; return map.hasOwnProperty(key) ? map[key] : undefined;
} }
static set<V>(map: {[key: string]: V}, key: string, value: V) { map[key] = value; } static set<V>(map: {[key: string]: V}, key: string, value: V) { map[key] = value; }
static keys(map: {[key: string]: any}): string[] { return Object.keys(map); } static keys(map: {[key: string]: any}): string[] { return Object.keys(map); }
static values<T>(map: {[key: string]: T}): T[] { static values<T>(map: {[key: string]: T}): T[] {
return Object.keys(map).reduce((r, a) => { return Object.keys(map).map((k: string): T => map[k]);
r.push(map[a]);
return r;
}, []);
} }
static isEmpty(map: {[key: string]: any}): boolean { static isEmpty(map: {[key: string]: any}): boolean {
for (var prop in map) { for (var prop in map) {
@ -130,27 +128,21 @@ export class StringMapWrapper {
return true; return true;
} }
static delete (map: {[key: string]: any}, key: string) { delete map[key]; } static delete (map: {[key: string]: any}, key: string) { delete map[key]; }
static forEach<K, V>(map: {[key: string]: V}, callback: /*(V, K) => void*/ Function) { static forEach<K, V>(map: {[key: string]: V}, callback: (v: V, K: string) => void) {
for (var prop in map) { for (let k of Object.keys(map)) {
if (map.hasOwnProperty(prop)) { callback(map[k], k);
callback(map[prop], prop);
}
} }
} }
static merge<V>(m1: {[key: string]: V}, m2: {[key: string]: V}): {[key: string]: V} { static merge<V>(m1: {[key: string]: V}, m2: {[key: string]: V}): {[key: string]: V} {
var m: {[key: string]: V} = {}; var m: {[key: string]: V} = {};
for (var attr in m1) { for (let k of Object.keys(m1)) {
if (m1.hasOwnProperty(attr)) { m[k] = m1[k];
m[attr] = m1[attr];
}
} }
for (var attr in m2) { for (let k of Object.keys(m2)) {
if (m2.hasOwnProperty(attr)) { m[k] = m2[k];
m[attr] = m2[attr];
}
} }
return m; return m;

View File

@ -539,7 +539,9 @@ export class FormGroup extends AbstractControl {
} }
/** @internal */ /** @internal */
_forEachChild(cb: Function): void { StringMapWrapper.forEach(this.controls, cb); } _forEachChild(cb: (v: any, k: string) => void): void {
StringMapWrapper.forEach(this.controls, cb);
}
/** @internal */ /** @internal */
_setParentForControls() { _setParentForControls() {