patternjavascriptMinor
Truly private JavaScript variables
Viewed 0 times
javascriptvariablesprivatetruly
Problem
I created a function which has, I believe, truly private variables. It's not quite production ready for several reasons:
Anyway, I'm putting this out there for critique / suggestions / recommendations for how to fix the above points. I'm not sure I'll ever really do anything with it, but I find it to be an interesting problem:
- It's using
Object.defineProperty-- I can deal with this
- It is re-defining every function in the prototype by doing
eval( fn.toString() )-- I do not like this
- I'm re-defining the prototypes, hence doing-away with the advantage of prototypes! -- deal breaker
Anyway, I'm putting this out there for critique / suggestions / recommendations for how to fix the above points. I'm not sure I'll ever really do anything with it, but I find it to be an interesting problem:
function foo(){
this.definePrivateProperties('foo', 'bar');
}
foo.prototype.setFoo = function(x){
this.foo = x;
};
foo.prototype.getFoo = function(){
return this.foo;
};
foo.prototype.definePrivateProperties = function(){
var self = this,
args = arguments,
validFns = {},
proto = this.__proto__;
// replace all functions in prototype with new ones!
// set a random key on each new function
// (not 100% secure, but close enough for now)
// this ensures that the function is unique to -this- object
for(var fn in proto){
this[ fn ] = eval( '(' + proto[fn] + ')' );
validFns[ this[ fn ].key = Math.random() ] = 1;
}
for(var i=0, l=args.length; i var x = new foo();
> x.setFoo(42);
> x.foo = 20;
20
> x.foo;
undefined
> x.getFoo();
42Solution
As for the code:
-
The property getter and setter should probably throw an exception if the check against the caller failed. If you attempt to set it from any function besides one defined on the prototype, you don't want it to silently fail -- if it does, you'll be wasting lots of time trying to figure out why your changes aren't sticking. (Particularly in light of other issues below.)
-
If you're going to assign a different key to each member function, you might consider making
-
You're setting keys on each thing you copy from the prototype, regardless of whether it's a function. Only functions need keys; other stuff (well, anything really, but particularly non-function members) might have its own purpose for
-
The way the access checks are now, only member functions can use them. Though that may be the intent, it is way more crippling than it sounds.
Observe:
We're in a private function, which should have access to
Say:
(BTW, see what we just did there? We can add a function and give it access to the "private" properties pretty much at will.) In order to prevent that...i dunno. redefine
In contrast, the standard method (hiding the variable away in a closure) is not prone to such hackery.
-
The property getter and setter should probably throw an exception if the check against the caller failed. If you attempt to set it from any function besides one defined on the prototype, you don't want it to silently fail -- if it does, you'll be wasting lots of time trying to figure out why your changes aren't sticking. (Particularly in light of other issues below.)
-
If you're going to assign a different key to each member function, you might consider making
validFns[key] be the function with that key, rather than just '1', so you can check it against the caller. (Ensures the key doesn't ever accidentally match.)-
You're setting keys on each thing you copy from the prototype, regardless of whether it's a function. Only functions need keys; other stuff (well, anything really, but particularly non-function members) might have its own purpose for
key.-
The way the access checks are now, only member functions can use them. Though that may be the intent, it is way more crippling than it sounds.
Observe:
foo.prototype.doStuff = function() {
var self = this;
$('input.some_selector').each(function() {
self.foo += parseInt(this.value);
});
};We're in a private function, which should have access to
foo. But since the actual access is within an anonymous function, and not doStuff, it's not allowed. In order to fix this (and fix it you should), your access check would need to go down the call stack (check the caller, then the caller's caller, and so on) til it finds a function with a key. Recursive functions seem to cause such a search massive amounts of trouble, though; a depth limit might help with that.definePrivatePropertiescan be run a second time. And a third, fourth, etc. And each time you call it, it obliterates the previous property's value in order to add the new one. Read: external code can mess up your private variables.
Say:
foo.prototype.mangleFoo = function() { this.foo = 100; };
x.definePrivateProperties('foo');
delete foo.prototype.mangleFoo;
x.mangleFoo();(BTW, see what we just did there? We can add a function and give it access to the "private" properties pretty much at will.) In order to prevent that...i dunno. redefine
definePrivateProperties after it's run once?In contrast, the standard method (hiding the variable away in a closure) is not prone to such hackery.
Code Snippets
foo.prototype.doStuff = function() {
var self = this;
$('input.some_selector').each(function() {
self.foo += parseInt(this.value);
});
};foo.prototype.mangleFoo = function() { this.foo = 100; };
x.definePrivateProperties('foo');
delete foo.prototype.mangleFoo;
x.mangleFoo();Context
StackExchange Code Review Q#3319, answer score: 6
Revisions (0)
No revisions yet.