[#25] Combined buildGetter() and buildSetter()

This helped to get rid of some unnecessary duplicate code and should also help
to improve performance slightly for getter/setter definitions.

lib/ClassBuilder.js

@@ -469,12 +469,8 @@ exports.prototype.buildMembers = function buildMembers(
         {
             var dest = ( keywordStatic( keywords ) ) ? smethods : members;
 
-            _self._memberBuilder.buildGetter(
+            _self._memberBuilder.buildGetterSetter(
-                dest, null, name, get, keywords, base
+                dest, null, name, get, set, keywords, base
-            );
-
-            _self._memberBuilder.buildSetter(
-                dest, null, name, set, keywords, base
             );
         },
 

lib/FallbackMemberBuilder.js

@@ -52,26 +52,13 @@ module.exports.constructor = module.exports;
 
 
 /**
- * Getters are unsupported in a pre-ES5 environment
+ * Getters/setters are unsupported in a pre-ES5 environment
  *
  * Simply throw an exception, as it clearly represents that the developer did
  * not account for the possibility that their software may have been executed in
  * a pre-ES5 environment.
  */
-exports.prototype.buildGetter = function()
+exports.prototype.buildGetterSetter = function()
 {
-    throw Error( 'Getters are unsupported in this environment' );
+    throw Error( 'Getters/setters are unsupported in this environment' );
-};
-
-
-/**
- * Setters are unsupported in a pre-ES5 environment
- *
- * Simply throw an exception, as it clearly represents that the developer did
- * not account for the possibility that their software may have been executed in
- * a pre-ES5 environment.
- */
-exports.prototype.buildSetter = function()
-{
-    throw Error( 'Setters are unsupported in this environment' );
 };

lib/MemberBuilder.js

@@ -187,72 +187,16 @@ exports.buildProp = function( members, meta, name, value, keywords, base )
 };
 
 
-/**
- * Copies a getter to the appropriate member prototype, depending on
- * visibility, and assigns necessary metadata from keywords
- *
- * XXX: Combine with buildSetter for performance benefit
- *
- * @param  {{public: Object, protected: Object, private: Object}} members
- *
- * @param  {Object}  meta   metadata container
- * @param  {string}  name   getter name
- * @param  {*}       value  getter value
- *
- * @param  {Object.<string,boolean>}  keywords  parsed keywords
- *
- * @param  {Object=}  base  optional base object to scan
- *
- * @return  {undefined}
- */
-exports.buildGetter = function( members, meta, name, value, keywords, base )
-{
-
-    this._buildGetterSetter(
-        'get', members, meta, name, value, keywords, base
-    );
-};
-
-
-/**
- * Copies a setter to the appropriate member prototype, depending on
- * visibility, and assigns necessary metadata from keywords
- *
- * XXX: Combine with buildGetter for performance benefit
- *
- * @param  {{public: Object, protected: Object, private: Object}} members
- *
- * @param  {Object}  meta   metadata container
- * @param  {string}  name   setter name
- * @param  {*}       value  setter value
- *
- * @param  {Object.<string,boolean>}  keywords  parsed keywords
- *
- * @param  {Object=}  base  optional base object to scan
- *
- * @return  {undefined}
- */
-exports.buildSetter = function( members, meta, name, value, keywords, base )
-{
-    this._buildGetterSetter(
-        'set', members, meta, name, value, keywords, base
-    );
-};
-
-
 /**
  * Copies a getter/setter to the appropriate member prototype, depending on
  * visibility, and assigns necessary metadata from keywords
  *
- * XXX: Combine getter/setter operations for performance benefit
- *
- * @param  {string}  type  get/set
- *
  * @param  {{public: Object, protected: Object, private: Object}} members
  *
  * @param  {Object}  meta  metadata container
  * @param  {string}  name  getter name
- * @param  {*}       value  getter value
+ * @param  {*}       get   getter value
+ * @param  {*}       set   setter value
  *
  * @param  {Object.<string,boolean>}  keywords  parsed keywords
  *
@@ -260,8 +204,8 @@ exports.buildSetter = function( members, meta, name, value, keywords, base )
  *
  * @return  {undefined}
  */
-exports._buildGetterSetter = function(
+exports.buildGetterSetter = function(
-    type, members, meta, name, value, keywords, base
+    members, meta, name, get, set, keywords, base
 )
 {
     var prev_data = scanMembers( members, name, base );
@@ -270,21 +214,15 @@ exports._buildGetterSetter = function(
         name, keywords, prev_data
     );
 
-    // the actual data to be set
-    var data = {
-        enumerable: true,
-
-        // otherwise we can't add a getter to this
-        configurable: true,
-    };
-
-    // set getter/setter appropriately, depending on the provided type
-    data[ type ] = value;
-
     Object.defineProperty(
         getMemberVisibility( members, keywords, name ),
         name,
-        data
+        {
+            get: get,
+            set: set,
+            enumerable:   true,
+            configurable: false,
+        }
     );
 };
 

test/FallbackMemberBuilderTest.js

@@ -50,33 +50,19 @@ require( './common' ).testCase(
     /**
      * Getters and setters are unsupported in pre-ES5 environments
      */
-    'buildGetter() and buildSetter() methods throw exceptions': function()
+    'buildGetterSetter() method throws an exception': function()
     {
         // getter test
         try
         {
-            this.sut.buildGetter();
+            this.sut.buildGetterSetter();
-            this.fail( 'Exception should have been called (getter)' );
+            this.fail( 'Exception should have been called (getter/setter)' );
         }
         catch ( e )
         {
             this.assertOk(
                 e.message.match( /unsupported/ ),
-                'Incorrect exception thrown (getter)'
+                'Incorrect exception thrown (getter/setter)'
-            );
-        }
-
-        // setter test
-        try
-        {
-            this.sut.buildSetter();
-            this.fail( 'Exception should have been called (getter)' );
-        }
-        catch ( e )
-        {
-            this.assertOk(
-                e.message.match( /unsupported/ ),
-                'Incorrect exception thrown (setter)'
             );
         }
     },

test/MemberBuilder/VisibilityTest.js

@@ -62,22 +62,16 @@ require( 'common' ).testCase(
         };
 
 
-        this.buildStubGetterSetter = function( name, val, visibility, type )
+        this.buildStubGetterSetter = function( name, get, set, visibility )
         {
-            var keywords = {},
+            var keywords = {};
-
-                // we can use bind() here because these tests will only be run
-                // in an ES5 environment (since pre-ES5 doesn't support
-                // getters/setters)
-                method = ( ( type === 'get' )
-                    ? _self.sut.buildGetter.bind( _self.sut )
-                    : _self.sut.buildSetter.bind( _self.sut )
-                )
-            ;
 
             // set visibility level using access modifier
             keywords[ visibility ] = true;
-            method( _self.members, {}, name, val, keywords, {} );
+
+            _self.sut.buildGetterSetter(
+                _self.members, {}, name, get, set, keywords, {}
+            );
         };
 
 
@@ -152,8 +146,7 @@ require( 'common' ).testCase(
             ;
 
             // build both the getter and the setter
-            _self.buildStubGetterSetter( name, getval, vis, 'get' );
+            _self.buildStubGetterSetter( name, getval, setval, vis, 'get' );
-            _self.buildStubGetterSetter( name, setval, vis, 'set' );
 
             // get the getter/setter
             var data = Object.getOwnPropertyDescriptor(
@@ -320,8 +313,9 @@ require( 'common' ).testCase(
         // getter/setter if supported
         if ( gst )
         {
-            this.sut.buildGetter( this.members, {}, name_gs, getval, {}, {} );
+            this.sut.buildGetterSetter(
-            this.sut.buildSetter( this.members, {}, name_gs, setval, {}, {} );
+                this.members, {}, name_gs, getval, setval, {}, {}
+            );
         }
 
         this.assertStrictEqual(
@@ -382,7 +376,7 @@ require( 'common' ).testCase(
     },
 
 
-    'Only one access modifier may be used per getter': function()
+    'Only one access modifier may be used per getter/setter': function()
     {
         if ( !gst ) return;
 
@@ -390,23 +384,9 @@ require( 'common' ).testCase(
 
         this.multiVisFailureTest( function( name, keywords )
         {
-            _self.sut.buildGetter(
+            _self.sut.buildGetterSetter(
-                _self.members, {}, name, function() {}, keywords, {}
+                _self.members, {}, name,
-            );
+                function() {}, function() {}, keywords, {}
-        } );
-    },
-
-
-    'Only one access modifier may be used per setter': function()
-    {
-        if ( !gst ) return;
-
-        var _self = this;
-
-        this.multiVisFailureTest( function( name, keywords )
-        {
-            _self.sut.buildSetter(
-                _self.members, {}, name, function() {}, keywords, {}
             );
         } );
     },