Prototype

Comments and changes to this ticket

• 

  Juriy Zaytsev June 28th, 2008 @ 03:25 AM

  I'd like some opinions on this one:

  
  // the fastest way
  // yields true when key matches "native" methods in the proto-chain
  // such as toString, valueOf, etc.
  Hash.prototype.include = function(key) {
    return key in this._object;
  }
  
  // #get filters Object.prototype.* methods
  // no _each is being used so should be fast as well
  Hash.prototype.include = function(key) {
    return !Object.isUndefined(this.get(key));
  }
  
  // "standard" yet "slow" way to check for a key
  // do we even need to go through such overhead?
  Hash.prototype.include = function(key) {
    return !Object.isUndefined(this.find(function(pair) {
      return pair.key == key;
    }))
  }
  
  

  I'm all for #2 - it's fast and bullet-proof

• 

  John-David Dalton June 28th, 2008 @ 07:07 AM

  Number 2 seems like the best choice.

  Number 3 is an example of what not to do.

  I would hope thats not the "standard" way to do something :P

  +1 for options :)

• 

  Andrew Dupont June 30th, 2008 @ 01:37 AM

  Seconding option 2.

• 

  Andrew Dupont June 30th, 2008 @ 01:38 AM

  • → State changed from “new” to “enhancement”
• 

  Tobie Langel June 30th, 2008 @ 04:49 AM

  >>> Hash.prototype.include = function(key) { return !Object.isUndefined(this.get(key)); }
  
  >>> $H({foo: undefined}).include('foo')
  
  false
  
  >>> Hash.prototype.include = function(key) { return key in this._object; }
  
  >>> $H({foo: undefined}).include('foo')
  
  true

  I'd favor the second one, makes more sense to me.

• 

  Juriy Zaytsev June 30th, 2008 @ 02:05 PM

  Tobie, that's a good point.

  Checking for key being undefined doesn't really mean that key is not present. As a matter of fact, it will still be revealed via enumeration until explicitly delete'ed. That means that Hash#keys would actually show the key (yet Hash#include would claim otherwise)

  Hash.prototype.include = function(key) { 
    return !Object.isUndefined(this.get(key)); 
  }
  var h = $H({ foo: undefined });
  h.include('foo'); // false
  h.keys(); // ['foo']
  

  This wouldn't happen if we were to use for/in (3rd option) but going from O(1) to O(N) is not too exciting : /

  #hasOwnProperty solves the problem nicely but we can't use it because of an old Safari

  Hash.prototype.include = function(key) { 
    return this._object.hasOwnProperty(key);
  }
  var h = $H({ foo: undefined });
  h.include('foo'); // true
  h.keys(); // ['foo']
  h.include('toString'); // false
  
  

  We could also try to simulate #hasOwnProperty (since we know that this._object inherits directly from Object.prototype):

  Hash.prototype.include = function(key) { 
    return key in this._object && !(key in Object.prototype); 
  }
  

• 

  John-David Dalton June 30th, 2008 @ 04:29 PM

  I liked option number 2 because it did the poor mans hasOwnProperty check.

  So I guess to get around it we can do:

  Hash.prototype.include = function(key) { 
    return key in this._object &&
      this._object[key] !== Object.prototype[key]; 
  };
  

  Also Juriy I think your check (in your previous comment) would skip methods like toString and valueOf because your check will fail if those are present in the Object.prototype, even if they are manually added to the keys of the _object.

  As to the hasOwnProperty. I don't see the big issue in making an Object.prototype.hasOwnProperty for Safari 2.0.4 and lower. It is skipped by for-in loops and the like. We can either do so via a browser sniff or a capability check. Making hasOwnProperty work for that older browser is easy and I think we should do it (it makes a lot of things less hacky because we can then use hasOwnProperty).

• 

  Juriy Zaytsev June 30th, 2008 @ 08:02 PM

  Last time I talked to Tobie, simulating hasOwnProperty was out of the question. I actually think we DO need to have that method. Having these workarounds all over the source is becoming ugly (and not all of them are fixed yet).

  Good catch, John, my last implementation will fail if hash has a method named as one of the Object.prototype.* ones. Your example will fail too when both property of a hash and Object.prototype.* one are undefined:

  Hash.prototype.include = function(key) { 
    return key in this._object &&
      this._object[key] !== Object.prototype[key]; 
  };
  
  var h = $H({ foo: undefined, toString: function(){ return 'foo' } });
  
  h.include('foo'); // false
  

  We should either use #hasOwnProperty or go with this crazy snippet:

  Hash.prototype.include = function(key) { 
    return key in this._object && 
      (!(key in Object.prototype) || Object.prototype[key] !== this._object[key]); 
  }
  
  var h = $H({ foo: undefined, toString: function(){ return 'foo' } });
  
  h.include('foo'); // true
  h.include('bar'); // false
  h.include('toString'); // true
  h.include('valueOf'); // false
  

  This is fun : )

• 

  John-David Dalton June 30th, 2008 @ 08:43 PM

  I totally vote for hasOwnProperty method for Safari 2.0.4 :)

  Thanks for the corrections Juriy.

• 

  Juriy Zaytsev July 23rd, 2008 @ 04:15 PM

  • no changes were found...

  • 0010-Subclass-Hash-include.patch 2.1 KB