From 124466e71945a4515a7b5742310594e8753c4314 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Mon, 29 Dec 2014 11:41:18 +0200 Subject: [PATCH] fix($mdUtil): fix bugs in iterator's `next()`/`previous()` methods Refactor for DRY-ness `next()` and `previous()` --- src/core/util/iterator.js | 87 +++++++---------- src/core/util/iterator.spec.js | 166 ++++++++++++++++++++++++++++++++- 2 files changed, 201 insertions(+), 52 deletions(-) diff --git a/src/core/util/iterator.js b/src/core/util/iterator.js index 65fd2701249..3d1846f89d0 100644 --- a/src/core/util/iterator.js +++ b/src/core/util/iterator.js @@ -44,8 +44,8 @@ first: first, last: last, - next: next, - previous: previous, + next: angular.bind(null, findSubsequentItem, false), + previous: angular.bind(null, findSubsequentItem, true), hasPrevious: hasPrevious, hasNext: hasNext @@ -169,48 +169,6 @@ return item && (indexOf(item) > -1); } - /** - * Find the next item. If reloop is true and at the end of the list, it will - * go back to the first item. If given ,the `validate` callback will be used - * determine whether the next item is valid. If not valid, it will try to find the - * next item again. - * @param item - * @param {optional} validate function - * @param {optional} recursion limit - * @returns {*} - */ - function next(item, validate, limit) { - validate = validate || trueFn; - - var index = indexOf(item) + 1; - var found = inRange(index) ? _items[ index ] : (reloop ? first() : null); - - found = hasCheckedAll(found, limit) ? null : found; - - return !found || validate(found) ? found : next(found, validate, limit || index); - } - - /** - * Find the previous item. If reloop is true and at the beginning of the list, it will - * go back to the last item. If given ,the `validate` callback will be used - * determine whether the previous item is valid. If not valid, it will try to find the - * previous item again. - * @param item - * @param {optional} validation function - * @param {optional} recursion limit - * @returns {*} - */ - function previous(item, validate, limit) { - validate = validate || trueFn; - - var index = indexOf(item) - 1; - var found = inRange(index) ? _items[ index ] : (reloop ? last() : null); - - found = hasCheckedAll(found, limit) ? null : found; - - return !found || validate(found) ? found : previous(found, validate, limit || index); - } - /** * Return first item in the list * @returns {*} @@ -228,15 +186,42 @@ } /** - * Has the iteration checked all items in the list - * @param item current found item in th list - * @param {optional} stopAt index - * @returns {*|boolean} + * Find the next item. If reloop is true and at the end of the list, it will + * go back to the first item. If given ,the `validate` callback will be used + * determine whether the next item is valid. If not valid, it will try to find the + * next item again. + * @param item + * @param {optional} validate function + * @param {optional} recursion limit + * @returns {*} */ - function hasCheckedAll(item, stopAt) { - return stopAt && item && (indexOf(item) == stopAt); - } + function findSubsequentItem(backwards, item, validate, limit) { + validate = validate || trueFn; + + var curIndex = indexOf(item); + if (!inRange(curIndex)) { + return null; + } + + var nextIndex = curIndex + (backwards ? -1 : 1); + var foundItem = null; + if (inRange(nextIndex)) { + foundItem = _items[nextIndex]; + } else if (reloop) { + foundItem = backwards ? last() : first(); + nextIndex = indexOf(foundItem); + } + if ((foundItem === null) || (nextIndex === limit)) { + return null; + } + + if (angular.isUndefined(limit)) { + limit = nextIndex; + } + + return validate(foundItem) ? foundItem : findSubsequentItem(backwards, foundItem, validate, limit); + } } })(); diff --git a/src/core/util/iterator.spec.js b/src/core/util/iterator.spec.js index 310c10a3893..ff0483d6b8b 100644 --- a/src/core/util/iterator.spec.js +++ b/src/core/util/iterator.spec.js @@ -236,6 +236,7 @@ describe('iterator', function() { expect( iter.next(iter.itemAt(index)) ).toBe('Max'); expect( iter.next(99) ).toBeNull(); + expect( iter.next(null) ).toBeNull(); }); it('should use previous() properly', function () { @@ -251,7 +252,6 @@ describe('iterator', function() { expect( iter.previous(99) ).toBe('Max'); expect( iter.previous(null) ).toBeNull(); - }); }); @@ -283,6 +283,170 @@ describe('iterator', function() { }); + describe('use to provide navigation API with relooping', function () { + var list, iter; + + beforeEach(inject(function ($mdUtil) { + list = [13, 14, 'Marcy', 15, 'Andrew', 16, 21, 'Adam', 37, 'Max', 99]; + iter = $mdUtil.iterator(list, true); + })); + + it('should use first() properly', inject(function ($mdUtil) { + expect(iter.first()).toBe(13); + + iter.add("47"); + expect(iter.first()).toBe(13); + + iter.add('Md', 0); + expect(iter.first()).toBe('Md'); + + iter.remove('Md'); + expect(iter.first()).toBe(13); + + iter.remove(iter.first()); + expect(iter.first()).toBe(14); + + iter = $mdUtil.iterator([2, 5]); + iter.remove(5); + expect(iter.first()).toBe(iter.last()); + })); + + it('should last() items properly', inject(function ($mdUtil) { + expect(iter.last()).toBe(99); + + iter.add("47"); + expect(iter.last()).toBe("47"); + + iter.add('Md', list.length); + expect(iter.last()).toBe('Md'); + + iter.remove('Md'); + expect(iter.last()).toBe("47"); + + iter.remove(iter.last()); + iter.remove(iter.last()); + expect(iter.last()).toBe('Max'); + + iter.remove(12); + expect(iter.last()).toBe('Max'); + expect(iter.first()).toBe(13); + + iter = $mdUtil.iterator([2, 5]); + iter.remove(2); + expect(iter.last()).toBe(iter.first()); + })); + + it('should use hasNext() properly', inject(function ($mdUtil) { + expect(iter.hasNext(iter.first())).toBe(true); + expect(iter.hasNext(iter.last())).toBe(false); + expect(iter.hasNext(99)).toBe(false); + expect(iter.hasNext('Andrew')).toBe(true); + + iter.add(100); + expect(iter.hasNext(99)).toBe(true); + iter.remove(100); + expect(iter.hasNext(99)).toBe(false); + + iter = $mdUtil.iterator(list = [2, 3]); + expect(iter.hasNext(iter.first())).toBe(true); + iter.remove(3); + expect(iter.hasNext(iter.first())).toBe(false); + expect(iter.hasNext(iter.last())).toBe(false); + + iter.remove(iter.last()); + expect(iter.count()).toBe(0); + expect(iter.hasNext(iter.last())).toBe(false); + + expect(iter.hasNext(null)).toBe(false); + })); + + it('should use hasPrevious() properly', inject(function ($mdUtil) { + expect(iter.hasPrevious(iter.first())).toBe(false); + expect(iter.hasPrevious(iter.last())).toBe(true); + expect(iter.hasPrevious(99)).toBe(true); + expect(iter.hasPrevious(13)).toBe(false); + expect(iter.hasPrevious('Andrew')).toBe(true); + + iter.add(100); + expect(iter.hasPrevious(99)).toBe(true); + iter.remove(100); + expect(iter.hasPrevious(99)).toBe(true); + + iter.remove(13); + expect(iter.hasPrevious(iter.first())).toBe(false); + + iter = $mdUtil.iterator(list = [2, 3]); + expect(iter.hasPrevious(iter.last())).toBe(true); + iter.remove(2); + expect(iter.hasPrevious(iter.last())).toBe(false); + expect(iter.hasPrevious(iter.first())).toBe(false); + + iter.remove(iter.first()); + expect(iter.count()).toBe(0); + expect(iter.hasPrevious(iter.first())).toBe(false); + + expect(iter.hasPrevious(null)).toBe(false); + })); + + it('should use next() properly', function () { + expect(iter.next(iter.first())).toBe(14); + + iter.add('47',0); + expect(iter.next(iter.first())).toBe(13); + + var index = list.length - 3; + expect(iter.next(iter.itemAt(index))).toBe('Max'); + + expect(iter.next(99)).toBe('47'); + expect(iter.next(null)).toBeNull(); + }); + + it('should use previous() properly', function () { + expect(iter.previous(iter.last())).toBe('Max'); + + iter.add('47',0); + expect(iter.previous(iter.itemAt(1))).toBe("47"); + + var index = list.length - 3; + expect(iter.previous(iter.itemAt(index))).toBe('Adam'); + + expect(iter.previous(99)).toBe('Max'); + expect(iter.previous('47')).toBe(99); + expect(iter.previous(null)).toBeNull(); + }); + }); + + describe('use to provide navigation API with relooping and validation', function () { + var list, iter; + var validate1 = function (item) { return (item !== 14) && (item !== 'Andrew'); }; + var validate2 = function () { return false; }; + + beforeEach(inject(function ($mdUtil) { + list = [13, 14, 'Marcy', 15, 'Andrew', 16, 21, 'Adam', 37, 'Max', 99]; + iter = $mdUtil.iterator(list, true); + })); + + it('should use next() properly', function () { + expect(iter.next(13, validate1)).toBe('Marcy'); + expect(iter.next('Marcy', validate1)).toBe(15); + expect(iter.next(15, validate1)).toBe(16); + expect(iter.next(99, validate1)).toBe(13); + + expect(iter.next(iter.first(), validate2)).toBeNull(); + expect(iter.next(iter.last(), validate2)).toBeNull(); + }); + + it('should use previous() properly', function () { + expect(iter.previous(16, validate1)).toBe(15); + expect(iter.previous(15, validate1)).toBe('Marcy'); + expect(iter.previous('Marcy', validate1)).toBe(13); + expect(iter.previous(13, validate1)).toBe(99); + + expect(iter.previous(iter.last(), validate2)).toBeNull(); + expect(iter.previous(iter.first(), validate2)).toBeNull(); + }); + }); + describe('use to provide a search API ', function () { var list, iter;