#192 enhancement
Juriy Zaytsev

Hash#include to work with keys

Reported by Juriy Zaytsev | June 27th, 2008 @ 04:06 AM | in 1.7

Hash includes Enumerable#include (via Enumerable). #include is not special-cased for Hash. It all boils down to the fact that method can never return true, since its (internally used) iterator is being passed an array - [key,value]. This dynamically generated array can never be equal to any other object passed into #include.

It's obviously possible to determine if instance contains certain key:

someHash.keys().include('something');

but that still leaves include somewhat useless (in a context of Hash)

Ruby's implementation iterates over keys: http://www.ruby-doc.org/core/cla...

I think we should follow this path (considering that there's already an #index method for values introspection).

Comments and changes to this ticket

  • Juriy Zaytsev

    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

    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
  • Andrew Dupont

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

    • State changed from “new” to “enhancement”
  • Tobie Langel

    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

    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

    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

    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

    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
  • T.J. Crowder

    T.J. Crowder November 16th, 2009 @ 04:50 PM

    • Tag changed from needs_patch, needs_tests to patched
    • Assigned user cleared.

    [responsible:none bulk edit command]

  • Tisho Georgiev

    Tisho Georgiev March 2nd, 2010 @ 09:32 AM

    • Tag changed from patched to needs:tests, patched, section:lang
  • sherymerykurian

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

The Prototype JavaScript library.

Shared Ticket Bins

Attachments

Pages