refactor(tagsInput): Encapsulate duplicate logic into a function

mbenford · Mar 10, 2015 · 5ecd0d4 · 5ecd0d4
1 parent 37148b5
commit 5ecd0d4
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 23 deletions.
diff --git a/src/tags-input.js b/src/tags-input.js
@@ -43,8 +43,7 @@
  */
 tagsInput.directive('tagsInput', function($timeout, $document, tagsInputConfig, tiUtil) {
 
-    function TagList(options, events, additionalCallbacks) {
-
+    function TagList(options, events, onTagAdding, onTagRemoving) {
         var self = {}, getTagText, setTagText, tagIsValid;
 
         getTagText = function(tag) {
@@ -57,18 +56,13 @@ tagsInput.directive('tagsInput', function($timeout, $document, tagsInputConfig,
 
         tagIsValid = function(tag) {
             var tagText = getTagText(tag);
-            var okToAddTag = additionalCallbacks.onTagAdding({ $tag: tag });
-            var valid = tagText &&
-                     tagText.length >= options.minLength &&
-                     tagText.length <= options.maxLength &&
-                     options.allowedTagsPattern.test(tagText) &&
-                     !tiUtil.findInObjectArray(self.items, tag, options.displayProperty);
-
-            if (okToAddTag !== undefined) {
-                valid = valid && okToAddTag;
-            }
 
-            return valid;
+            return tagText &&
+                   tagText.length >= options.minLength &&
+                   tagText.length <= options.maxLength &&
+                   options.allowedTagsPattern.test(tagText) &&
+                   !tiUtil.findInObjectArray(self.items, tag, options.displayProperty) &&
+                   onTagAdding({ $tag: tag });
         };
 
         self.items = [];
@@ -101,9 +95,8 @@ tagsInput.directive('tagsInput', function($timeout, $document, tagsInputConfig,
 
         self.remove = function(index) {
             var tag = self.items[index];
-            var okToRemoveTag = additionalCallbacks.onTagRemoving({ $tag: tag });
 
-            if (okToRemoveTag === undefined || okToRemoveTag)  {
+            if (onTagRemoving({ $tag: tag }))  {
                 self.items.splice(index, 1);
                 events.trigger('tag-removed', { $tag: tag });
                 return tag;
@@ -172,12 +165,9 @@ tagsInput.directive('tagsInput', function($timeout, $document, tagsInputConfig,
                 spellcheck: [Boolean, true]
             });
 
-            var additionalCallbacks = {
-                onTagAdding: $scope.onTagAdding,
-                onTagRemoving: $scope.onTagRemoving
-            };
-
-            $scope.tagList = new TagList($scope.options, $scope.events, additionalCallbacks);
+            $scope.tagList = new TagList($scope.options, $scope.events,
+                tiUtil.handleUndefinedResult($scope.onTagAdding, true),
+                tiUtil.handleUndefinedResult($scope.onTagRemoving, true));
 
             this.registerAutocomplete = function() {
                 var input = $element.find('input');

diff --git a/src/util.js b/src/util.js
@@ -69,6 +69,13 @@ tagsInput.factory('tiUtil', function($timeout) {
             .replace(/>/g, '&gt;');
     };
 
+    self.handleUndefinedResult = function(fn, valueIfUndefined) {
+        return function() {
+            var result = fn.apply(null, arguments);
+            return angular.isUndefined(result) ? valueIfUndefined : result;
+        };
+    };
+
     self.simplePubSub = function() {
         var events = {};
         return {
@@ -84,8 +91,7 @@ tagsInput.factory('tiUtil', function($timeout) {
             trigger: function(name, args) {
                 var handlers = events[name] || [];
                 handlers.every(function(handler) {
-                    var retVal = handler.call(null, args);
-                    return angular.isUndefined(retVal) || retVal;
+                    return self.handleUndefinedResult(handler, true)(args);
                 });
                 return this;
             }

diff --git a/test/util.spec.js b/test/util.spec.js
@@ -197,4 +197,38 @@ describe('tiUtil factory', function() {
         });
     });
 
+    describe('handleUndefinedResult', function() {
+        it('wraps the provided function and change its return value if it\'s undefined', function() {
+            // Arrange
+            var fn = function() { return; };
+
+            // Act
+            var result = tiUtil.handleUndefinedResult(fn, 'foobar')();
+
+            // Act/Assert
+            expect(result).toBe('foobar');
+        });
+
+        it('wraps the provided function and does not change its return value if it\'s defined', function() {
+            // Arrange
+            var fn = function() { return 1; };
+
+            // Act
+            var result = tiUtil.handleUndefinedResult(fn, 'foobar')();
+
+            // Act/Assert
+            expect(result).toBe(1);
+        });
+
+        it('re-passes the provided arguments to the wrapped function', function() {
+            // Arrange
+            var fn = function(a, b) { return a + b; };
+
+            // Act
+            var result = tiUtil.handleUndefinedResult(fn)(1, 2);
+
+            // Assert
+            expect(result).toBe(3);
+        });
+    });
 });