Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Feature: Introduced skipping items when binging collections.
Browse files Browse the repository at this point in the history
  • Loading branch information
@oskarwrobel
oskarwrobel committed Jan 8, 2018
1 parent cc96644 commit 79ee2fc
Show file tree
Hide file tree
Showing 2 changed files with 251 additions and 8 deletions.
106 changes: 101 additions & 5 deletions src/collection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ export default class Collection {
*/
this._bindToInternalToExternalMap = new WeakMap();

this._skippedIndexesFromExternal = [];

/**
* A collection instance this collection is bound to as a result
* of calling {@link #bindTo} method.
Expand Down Expand Up @@ -271,7 +273,7 @@ export default class Collection {
this._bindToInternalToExternalMap.delete( item );
this._bindToExternalToInternalMap.delete( externalItem );

this.fire( 'remove', item );
this.fire( 'remove', item, index );

return item;
}
Expand Down Expand Up @@ -402,9 +404,28 @@ export default class Collection {
* console.log( target.get( 0 ).value ); // 'foo'
* console.log( target.get( 1 ).value ); // 'bar'
*
* It's possible to skip specified items by returning falsy value:
*
* const source = new Collection();
* const target = new Collection();
*
* target.bindTo( source ).using( item => {
* if ( item.hidden ) {
* return null;
* }
*
* return item;
* } );
*
* source.add( { hidden: true } );
* source.add( { hidden: false } );
*
* console.log( source.length ); // 2
* console.log( target.length ); // 1
*
* **Note**: {@link #clear} can be used to break the binding.
*
* @param {module:utils/collection~Collection} collection A collection to be bound.
* @param {module:utils/collection~Collection} externalCollection A collection to be bound.
* @returns {Object}
* @returns {module:utils/collection~Collection#bindTo#as} return.as
* @returns {module:utils/collection~Collection#bindTo#using} return.using
Expand Down Expand Up @@ -476,10 +497,70 @@ export default class Collection {
} else {
const item = factory( externalItem );

// When there is no item we need to remember skipped index first and then we can skip this item.
if ( !item ) {
this._skippedIndexesFromExternal.push( index );

return;
}

// Lets try to put item at the same index as index in external collection
// but when there are a skipped items in one or both collections we need to recalculate this index.
let finalIndex = index;

// When we try to insert item after some skipped items from external collection we need
// to include this skipped items and decrease index.
//
// For the following example:
// external -> [ 'A', 'B - skipped for internal', 'C - skipped for internal' ]
// internal -> [ A ]
//
// Another item is been added at the end of external collection:
// external.add( 'D' )
// external -> [ 'A', 'B - skipped for internal', 'C - skipped for internal', 'D' ]
//
// We can't just add 'D' to internal at the same index as index in external because
// this will produce empty indexes what is invalid:
// internal -> [ 'A', empty, empty, 'D' ]
//
// So we need to include skipped items and decrease index
// internal -> [ 'A', 'D' ]
for ( const skipped of this._skippedIndexesFromExternal ) {
if ( index > skipped ) {
finalIndex--;
}
}

// We need to take into consideration that external collection could skip some items from
// internal collection.
//
// For the following example:
// internal -> [ 'A', 'B - skipped for external', 'C - skipped for external' ]
// external -> [ A ]
//
// Another item is been added at the end of external collection:
// external.add( 'D' )
// external -> [ 'A', 'D' ]
//
// We need to include skipped items and place new item after them:
// internal -> [ 'A', 'B - skipped for external', 'C - skipped for external', 'D' ]
for ( const skipped of externalCollection._skippedIndexesFromExternal ) {
if ( finalIndex >= skipped ) {
finalIndex++;
}
}

this._bindToExternalToInternalMap.set( externalItem, item );
this._bindToInternalToExternalMap.set( item, externalItem );

this.add( item, index );
this.add( item, finalIndex );

// After adding new element to internal collection we need update indexes
// of skipped items in external collection.
for ( let i = 0; i < externalCollection._skippedIndexesFromExternal.length; i++ ) {
if ( finalIndex <= externalCollection._skippedIndexesFromExternal[ i ] ) {
externalCollection._skippedIndexesFromExternal[ i ]++;
}
}
}
};

Expand All @@ -492,12 +573,26 @@ export default class Collection {
this.listenTo( externalCollection, 'add', addItem );

// Synchronize the with collection as new items are removed.
this.listenTo( externalCollection, 'remove', ( evt, externalItem ) => {
this.listenTo( externalCollection, 'remove', ( evt, externalItem, index ) => {
const item = this._bindToExternalToInternalMap.get( externalItem );

if ( item ) {
this.remove( item );
}

// After removing element from external collection we need update/remove indexes
// of skipped items in internal collection.
this._skippedIndexesFromExternal = this._skippedIndexesFromExternal.reduce( ( result, skipped ) => {
if ( index < skipped ) {
result.push( skipped - 1 );
}

if ( index > skipped ) {
result.push( skipped );
}

return result;
}, [] );
} );
}

Expand All @@ -520,6 +615,7 @@ export default class Collection {
*
* @event remove
* @param {Object} item The removed item.
* @param {Number} index Index from which item was removed.
*/
}

Expand Down
153 changes: 150 additions & 3 deletions tests/collection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,9 +448,9 @@ describe( 'Collection', () => {
collection.remove( 'bom' ); // by id

sinon.assert.calledThrice( spy );
sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item1 );
sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item2 );
sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item3 );
sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item1, 0 );
sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item2, 1 );
sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item3, 0 );
} );

it( 'should throw an error on invalid index', () => {
Expand Down Expand Up @@ -767,6 +767,28 @@ describe( 'Collection', () => {
expect( collection.get( 0 ).value ).to.equal( 'foo' );
expect( collection.get( 1 ).value ).to.equal( 'bar' );
} );

it( 'skips when there is no item', () => {
collection.bindTo( items ).using( item => {
if ( item.skip ) {
return null;
}

return item;
} );

expect( collection ).to.have.length( 0 );

items.add( { value: 1, skip: false } );
items.add( { value: 2, skip: true } );
items.add( { value: 3, skip: false } );

expect( Array.from( collection, item => item.value ) ).to.deep.equal( [ 1, 3 ] );

items.add( { value: 4, skip: false }, 1 );

expect( Array.from( collection, item => item.value ) ).to.deep.equal( [ 1, 4, 3 ] );
} );
} );

describe( 'property name', () => {
Expand Down Expand Up @@ -802,6 +824,22 @@ describe( 'Collection', () => {
items.remove( 0 );
expect( collection ).to.have.length( 0 );
} );

it( 'skips when there is no item', () => {
collection.bindTo( items ).using( 'prop' );

expect( collection ).to.have.length( 0 );

items.add( { prop: { value: 'foo' } } );
items.add( { value: null } );
items.add( { prop: { value: 'biz' } } );

expect( Array.from( collection, item => item.value ) ).to.deep.equal( [ 'foo', 'biz' ] );

items.add( { prop: { value: 'bar' } }, 1 );

expect( Array.from( collection, item => item.value ) ).to.deep.equal( [ 'foo', 'bar', 'biz' ] );
} );
} );
} );

Expand Down Expand Up @@ -1032,6 +1070,115 @@ describe( 'Collection', () => {
sinon.assert.callCount( spyRemoveA, 2 );
sinon.assert.callCount( spyRemoveB, 2 );
} );

describe( 'skipping items', () => {
let collectionA, collectionB;

beforeEach( () => {
collectionA = new Collection();
collectionB = new Collection();

// A<--->B
collectionA.bindTo( collectionB ).using( item => {
if ( item.skip ) {
return null;
}

return item;
} );

collectionB.bindTo( collectionA ).using( item => {
if ( item.skip ) {
return null;
}

return item;
} );
} );

it( 'should add items at the enf of collections when includes skipped items', () => {
collectionA.add( { v: 'A' } );
collectionA.add( { v: 'B', skip: true } );
collectionA.add( { v: 'C', skip: true } );
collectionA.add( { v: 'D' } );

expect( collectionA._skippedIndexesFromExternal ).to.have.members( [] );
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
assertItems( collectionA, [ 'A', 'B', 'C', 'D' ] );
assertItems( collectionB, [ 'A', 'D' ] );

collectionB.add( { v: 'E' } );
collectionB.add( { v: 'F', skip: true } );
collectionB.add( { v: 'G' } );

expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3 ] );
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
assertItems( collectionA, [ 'A', 'B', 'C', 'D', 'E', 'G' ] );
assertItems( collectionB, [ 'A', 'D', 'E', 'F', 'G' ] );
} );

it( 'should add items between skipped items', () => {
collectionA.add( { v: 'A' } );
collectionA.add( { v: 'B', skip: true } );
collectionA.add( { v: 'C', skip: true } );
collectionA.add( { v: 'D' } );

collectionB.add( { v: 'E' } );
collectionB.add( { v: 'F', skip: true } );
collectionB.add( { v: 'G', skip: true } );
collectionB.add( { v: 'H' } );

expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3, 4 ] );
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
assertItems( collectionA, [ 'A', 'B', 'C', 'D', 'E', 'H' ] );
assertItems( collectionB, [ 'A', 'D', 'E', 'F', 'G', 'H' ] );

collectionA.add( { v: 'I' }, 2 );

expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 4, 5 ] );
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
assertItems( collectionA, [ 'A', 'B', 'I', 'C', 'D', 'E', 'H' ] );
assertItems( collectionB, [ 'A', 'I', 'D', 'E', 'F', 'G', 'H' ] );

collectionB.add( { v: 'J' }, 5 );

expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 4, 5 ] );
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
assertItems( collectionA, [ 'A', 'B', 'I', 'C', 'D', 'E', 'J', 'H' ] );
assertItems( collectionB, [ 'A', 'I', 'D', 'E', 'F', 'J', 'G', 'H' ] );
} );

it( 'should properly remove skipped items and update skipped indexes', () => {
collectionA.add( { v: 'A' } );
collectionA.add( { v: 'B', skip: true } );
collectionA.add( { v: 'C', skip: true } );
collectionA.add( { v: 'D' } );

collectionB.add( { v: 'E' } );
collectionB.add( { v: 'F', skip: true } );
collectionB.add( { v: 'G', skip: true } );
collectionB.add( { v: 'H' } );

expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3, 4 ] );
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
assertItems( collectionA, [ 'A', 'B', 'C', 'D', 'E', 'H' ] );
assertItems( collectionB, [ 'A', 'D', 'E', 'F', 'G', 'H' ] );

collectionA.remove( 2 );

expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3, 4 ] );
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1 ] );
assertItems( collectionA, [ 'A', 'B', 'D', 'E', 'H' ] );
assertItems( collectionB, [ 'A', 'D', 'E', 'F', 'G', 'H' ] );

collectionB.remove( 3 );

expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3 ] );
expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1 ] );
assertItems( collectionA, [ 'A', 'B', 'D', 'E', 'H' ] );
assertItems( collectionB, [ 'A', 'D', 'E', 'G', 'H' ] );
} );
} );
} );
} );

Expand Down

0 comments on commit 79ee2fc

Please sign in to comment.