Optimization on goog.object

[GoogleCodeExporter]

Hi, I've been using the closure library for almost a year now, and I would like 
to suggest updating the function goog.object.getCount()

from:

goog.object.getCount = function(obj) {
  // JS1.5 has __count__ but it has been deprecated so it raises a warning...
  // in other words do not use. Also __count__ only includes the fields on the
  // actual object and not in the prototype chain.
  var rv = 0;
  for (var key in obj) {
    rv++;
  }
  return rv;
};

to:

goog.object.getCount = function(obj) {
  Object.keys(obj).length
};

Thank you: avenancio@google.com (Andre Venancio)

Original issue reported on code.google.com by venancio...@gmail.com on 19 Mar 2013 at 4:44

[GoogleCodeExporter]

do you have any evidence that this is faster? looking at the v8 source code, i 
wouldn't expect it to be faster.

Original comment by Nicholas.J.Santos on 19 Mar 2013 at 7:14

[GoogleCodeExporter]

Hi Nicholas!
I just did a quick jsfiddle test to illustrate my suggestion.

http://jsfiddle.net/andrevenancio/AE3ks/

Original comment by venancio...@gmail.com on 19 Mar 2013 at 9:11

[GoogleCodeExporter]

Do you guys usually provide feedback?

Original comment by venancio...@gmail.com on 28 Mar 2013 at 5:57

[GoogleCodeExporter]

Object.keys is not in JS 1.5

https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Obj
ect/keys

There could be an optimization to use it if present.

Original comment by nn...@google.com on 12 Apr 2013 at 8:25

• Added labels: Priority-Low
• Removed labels: Priority-Medium

[GoogleCodeExporter]

Is this the right file   

Original comment by jina.son...@gmail.com on 27 May 2013 at 7:52

Attachments:

• smile.JPG

[GoogleCodeExporter]

This optimization looks nice in theory, but unfortunately it changes the 
semantics of goog.object.getCount if Object.prototype is altered.

Object.prototype.f = function() {};
Object.keys({}).length == 0
goog.object.getCount({}) == 1

Tracking down whether it breaks existing projects is close to impossible.

Original comment by pall...@google.com on 27 May 2013 at 1:10

• Changed state: WontFix
• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

(I really like the smile file on the top)
Regarding your answer, I agree to disagree. Firstly prototyping the global 
Object just doesn't look a good OOP approach. But, lets imagine we do it, If 
you have this:

goog.object.getCount = function(obj) {
  // JS1.5 has __count__ but it has been deprecated so it raises a warning...
  // in other words do not use. Also __count__ only includes the fields on the
  // actual object and not in the prototype chain.
  /*var rv = 0;
  for (var key in obj) {
    rv++;
  }
  return rv;*/
  return Object.keys(obj).length;
};

And you call this:

var obj = {};
Object.prototype.f = function(){};
console.log(goog.object.getCount(obj))

You will still get a 0 as length....

So, what am I missing? The only valid answer might actually be: it wont work in 
browsers that have javascript version older then 1.8.5

Original comment by venancio...@gmail.com on 30 May 2013 at 9:20

[GoogleCodeExporter]

The original implementation returns 1 as length. Object.keys(obj).length 
returns 0.
It's very hard to verify whether any projects depend on the old behavior.

Adding stuff to Object.prototype is indeed dangerous but not uncommon. AFAIK 
the Prototype library used to do that.

Also, if we update goog.object.getCount to only return the object's own keys, 
we should do the same with all other functions in the goog.object namespace at 
the same time to avoid inconsistencies.

Original comment by pall...@google.com on 30 May 2013 at 9:37

[GoogleCodeExporter]

Thanks for your answer, I didn't knew AFAIK, I'll have a look at it.
Regarding your answer, knockout by exaustion in my tests, I always get 0,

goog.object.getCount({}) --> Zero! :)

Original comment by venancio...@gmail.com on 30 May 2013 at 11:41

[GoogleCodeExporter]

As an experiment I added an assertion to the goog.structs.StringSet constructor 
which ensures that Object.prototype is not augmented with enumerable keys.
If the experiment succeeds, we may try the same with goog.object.getKeys and 
getCount. Then we can benefit from the faster Object.keys function.

Original comment by pall...@google.com on 27 Jun 2013 at 12:09

• Changed state: New

[GoogleCodeExporter]

Great news! Hope you're successful :)

Original comment by venancio...@gmail.com on 5 Jul 2013 at 11:57

[GoogleCodeExporter]

Unfortunately not.

Some callers of goog.object.getKeys expect that the keys from the prototype are 
also returned. One example is testing/mock.js.

goog.object.getCount would be easier to change, but breaking the  
goog.object.getCount(obj) == goog.object.getKeys(obj).length invariant may be 
pretty dangerous.

Original comment by pall...@google.com on 12 Jul 2013 at 1:08

• Changed state: WontFix