From 5797ed60f5c2f0c4b21f6c7658304e5ef91d792b Mon Sep 17 00:00:00 2001 From: danielo Date: Wed, 12 Jul 2017 13:31:27 +0100 Subject: [PATCH 1/3] formatting changes to observable class --- .../br-util/src/br/util/Observable.js | 39 ++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/brjs-sdk/sdk/libs/javascript/br-util/src/br/util/Observable.js b/brjs-sdk/sdk/libs/javascript/br-util/src/br/util/Observable.js index a3ce0b2f4..898197b7d 100644 --- a/brjs-sdk/sdk/libs/javascript/br-util/src/br/util/Observable.js +++ b/brjs-sdk/sdk/libs/javascript/br-util/src/br/util/Observable.js @@ -1,10 +1,10 @@ -'use strict'; +"use strict"; /** * @module br/util/Observable */ -var Errors = require('br/Errors'); +var Errors = require("br/Errors"); /** * Constructs a new Observable. @@ -66,13 +66,16 @@ Observable.prototype.getCount = function() { * String, Number, Boolean or Function. */ Observable.prototype.addObserver = function(observer) { - if (!(observer instanceof Object) || + if ( + !(observer instanceof Object) || (observer instanceof String || - observer instanceof Number || - observer instanceof Boolean || - observer instanceof Function)) { - - throw new Errors.InvalidParametersError('An observer must be an object'); + observer instanceof Number || + observer instanceof Boolean || + observer instanceof Function) + ) { + throw new Errors.InvalidParametersError( + "An observer must be an object" + ); } this.m_pObservers.push(observer); @@ -90,7 +93,7 @@ Observable.prototype.addObserver = function(observer) { * String, Number, Boolean or Function. */ Observable.prototype.addUniqueObserver = function(observer) { - var observerNotAdded = (this._getObserverIndex(observer) == -1); + var observerNotAdded = this._getObserverIndex(observer) == -1; if (observerNotAdded) { this.addObserver(observer); @@ -147,7 +150,7 @@ Observable.prototype.removeAllObservers = function() { this.m_pObservers = []; }; - /** +/** * Gets a list of all the observers that have been registered with this Observable. * * @return {Array} A list of the observers that have been registered. @@ -177,8 +180,10 @@ Observable.prototype.notifyObservers = function(methodName, parameters) { var observersCopy = this.m_pObservers.slice(); for (var idx = 0, len = observersCopy.length; idx < len; idx++) { var observer = observersCopy[idx]; - if (typeof observer[methodName] !== 'function') { - throw new Errors.NotSupportedError("Observer does not implement '" + methodName + "'"); + if (typeof observer[methodName] !== "function") { + throw new Errors.NotSupportedError( + "Observer does not implement '" + methodName + "'" + ); } observer[methodName].apply(observer, parameters); @@ -215,7 +220,11 @@ Observable.prototype.notify = function(methodName) { * * @see br.util.Observable#notifyObservers to notify without guarding against exceptions. */ -Observable.prototype.notifyObserversWithTryCatch = function(methodName, parameters, throwExceptions) { +Observable.prototype.notifyObserversWithTryCatch = function( + methodName, + parameters, + throwExceptions +) { if (!parameters) { parameters = []; } @@ -227,7 +236,9 @@ Observable.prototype.notifyObserversWithTryCatch = function(methodName, paramete try { observer[methodName].apply(observer, parameters); } catch (e) { - failedNotifications.push(new Observable.FailedNotification(observer, methodName, e)); + failedNotifications.push( + new Observable.FailedNotification(observer, methodName, e) + ); } } From a0fb76c92393ffa4ce6bae3a7be0278184b8f8e5 Mon Sep 17 00:00:00 2001 From: danielo Date: Wed, 12 Jul 2017 13:32:55 +0100 Subject: [PATCH 2/3] switching from use of instanceof to typeof in observable class to avoid errors when using other br libraries --- .../libs/javascript/br-util/src/br/util/Observable.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/brjs-sdk/sdk/libs/javascript/br-util/src/br/util/Observable.js b/brjs-sdk/sdk/libs/javascript/br-util/src/br/util/Observable.js index 898197b7d..408069650 100644 --- a/brjs-sdk/sdk/libs/javascript/br-util/src/br/util/Observable.js +++ b/brjs-sdk/sdk/libs/javascript/br-util/src/br/util/Observable.js @@ -67,11 +67,11 @@ Observable.prototype.getCount = function() { */ Observable.prototype.addObserver = function(observer) { if ( - !(observer instanceof Object) || - (observer instanceof String || - observer instanceof Number || - observer instanceof Boolean || - observer instanceof Function) + !(typeof observer === "object" && observer !== null) || + (typeof observer === "string" || + typeof observer === "number" || + typeof observer === "boolean" || + typeof observer === "function") ) { throw new Errors.InvalidParametersError( "An observer must be an object" From bf79e890147a05c729d61d0ff40e3bba9f2bdfb9 Mon Sep 17 00:00:00 2001 From: danielo Date: Wed, 12 Jul 2017 16:25:54 +0100 Subject: [PATCH 3/3] adding check for isNan case in addObserver check + removing tests for cases where new (String|Boolean|Number) is used as this is avoided by most developers --- .../br-util/src/br/util/Observable.js | 2 +- .../test-unit/tests/br/util/ObservableTest.js | 36 ++----------------- 2 files changed, 4 insertions(+), 34 deletions(-) diff --git a/brjs-sdk/sdk/libs/javascript/br-util/src/br/util/Observable.js b/brjs-sdk/sdk/libs/javascript/br-util/src/br/util/Observable.js index 408069650..259a70f7e 100644 --- a/brjs-sdk/sdk/libs/javascript/br-util/src/br/util/Observable.js +++ b/brjs-sdk/sdk/libs/javascript/br-util/src/br/util/Observable.js @@ -69,7 +69,7 @@ Observable.prototype.addObserver = function(observer) { if ( !(typeof observer === "object" && observer !== null) || (typeof observer === "string" || - typeof observer === "number" || + (typeof observer === "number" && !isNan(observer)) || typeof observer === "boolean" || typeof observer === "function") ) { diff --git a/brjs-sdk/sdk/libs/javascript/br-util/test-unit/tests/br/util/ObservableTest.js b/brjs-sdk/sdk/libs/javascript/br-util/test-unit/tests/br/util/ObservableTest.js index d9f19cf44..a798ffa1e 100644 --- a/brjs-sdk/sdk/libs/javascript/br-util/test-unit/tests/br/util/ObservableTest.js +++ b/brjs-sdk/sdk/libs/javascript/br-util/test-unit/tests/br/util/ObservableTest.js @@ -139,11 +139,7 @@ assertException(function() { oThis.m_oObservable.addObserver(10); }, "InvalidParametersError"); }; - ObservableTest.prototype.test_addNumericObserverFails = function() - { - var oThis = this; - assertException(function() { oThis.m_oObservable.addObserver(new Number(10)); }, "InvalidParametersError"); - }; + ObservableTest.prototype.test_addStringObserverFails = function() { @@ -151,11 +147,7 @@ assertException(function() { oThis.m_oObservable.addObserver("test"); }, "InvalidParametersError"); }; - ObservableTest.prototype.test_addNewStringObserverFails = function() - { - var oThis = this; - assertException(function() { oThis.m_oObservable.addObserver(new String("test")); }, "InvalidParametersError"); - }; + ObservableTest.prototype.test_addFunctionObserverFails = function() { @@ -170,11 +162,7 @@ assertException(function() { oThis.m_oObservable.addObserver(false); }, "InvalidParametersError"); }; - ObservableTest.prototype.test_addBooleanObserverFails = function() - { - var oThis = this; - assertException(function() { oThis.m_oObservable.addObserver(new Boolean(false)); }, "InvalidParametersError"); - }; + ObservableTest.prototype.test_addDuplicateObserverSucceeds = function() { @@ -213,24 +201,12 @@ assertException(function() { oThis.m_oObservable.addUniqueObserver(10); }, "InvalidParametersError"); }; - ObservableTest.prototype.test_addNumericUniqueObserverFails = function() - { - var oThis = this; - assertException(function() { oThis.m_oObservable.addUniqueObserver(new Number(10)); }, "InvalidParametersError"); - }; - ObservableTest.prototype.test_addStringUniqueObserverFails = function() { var oThis = this; assertException(function() { oThis.m_oObservable.addUniqueObserver("test"); }, "InvalidParametersError"); }; - ObservableTest.prototype.test_addNewStringUniqueObserverFails = function() - { - var oThis = this; - assertException(function() { oThis.m_oObservable.addUniqueObserver(new String("test")); }, "InvalidParametersError"); - }; - ObservableTest.prototype.test_addFunctionUniqueObserverFails = function() { var oThis = this; @@ -244,12 +220,6 @@ assertException(function() { oThis.m_oObservable.addUniqueObserver(false); }, "InvalidParametersError"); }; - ObservableTest.prototype.test_addBooleanUniqueObserverFails = function() - { - var oThis = this; - assertException(function() { oThis.m_oObservable.addUniqueObserver(new Boolean(false)); }, "InvalidParametersError"); - }; - ObservableTest.prototype.test_addDuplicateUniqueObserverFails = function() { this.m_oObserver1.expects(once()).method1();