Cleanup Module#attr_reader and Module#attr_writer

meh · meh · commit 7ab4d64076fc · 2015-02-24T19:40:23.000+01:00
diff --git a/opal/corelib/module.rb b/opal/corelib/module.rb
@@ -79,20 +79,33 @@ def attr_accessor(*names)
     attr_writer(*names)
   end
 
+  alias attr attr_accessor
+
   def attr_reader(*names)
     %x{
-      for (var i = 0, length = names.length; i < length; i++) {
-        (function(name) {
-          self.$$proto[name] = nil;
-          var func = function() { return this[name] };
+      var proto = self.$$proto;
 
-          if (self.$$is_singleton) {
-            self.$$proto.constructor.prototype['$' + name] = func;
-          }
-          else {
-            Opal.defn(self, '$' + name, func);
-          }
-        })(names[i]);
+      for (var i = names.length - 1; i >= 0; i--) {
+        var name = names[i],
+            id   = '$' + name;
+
+        // the closure here is needed because name will change at the next
+        // cycle, I wish we could use let.
+        var body = (function(name) {
+          return function() {
+            return this[name];
+          };
+        })(name);
+
+        // initialize the instance variable as nil
+        proto[name] = nil;
+
+        if (self.$$is_singleton) {
+          proto.constructor.prototype[id] = body;
+        }
+        else {
+          Opal.defn(self, id, body);
+        }
       }
     }
 
@@ -101,25 +114,35 @@ def attr_reader(*names)
 
   def attr_writer(*names)
     %x{
-      for (var i = 0, length = names.length; i < length; i++) {
-        (function(name) {
-          self.$$proto[name] = nil;
-          var func = function(value) { return this[name] = value; };
+      var proto = self.$$proto;
 
-          if (self.$$is_singleton) {
-            self.$$proto.constructor.prototype['$' + name + '='] = func;
-          }
-          else {
-            Opal.defn(self, '$' + name + '=', func);
+      for (var i = names.length - 1; i >= 0; i--) {
+        var name = names[i],
+            id   = '$' + name + '=';
+
+        // the closure here is needed because name will change at the next
+        // cycle, I wish we could use let.
+        var body = (function(name){
+          return function(value) {
+            return this[name] = value;
           }
-        })(names[i]);
+        })(name);
+
+        // initialize the instance variable as nil
+        proto[name] = nil;
+
+        if (self.$$is_singleton) {
+          proto.constructor.prototype[id] = body;
+        }
+        else {
+          Opal.defn(self, id, body);
+        }
       }
     }
+
     nil
   end
 
-  alias attr attr_accessor
-
   def autoload(const, path)
     %x{
       var autoloaders;
