memcached
memcached copied to clipboard
add if (!value.hasOwnProperty(vKey)) continue; that occurred bugs!
Before patching this patch, some functions attached to the object will be parsed and thrown errors!
For an example:
Array.prototype.foo = function() {};
var a = [];
for(var key in a) {
console.log(key);
}
The result will be abc
- this is not my expected result!
In your for ... in
of util.js:
case Array:
if (toString.call(value) !== '[object Array]') {
err = 'Argument "' + key + '" is not a valid Array.';
}
if (!err && key === 'key') {
for (var vKey in value) {
...
We should ignore abc
this extended function.
Some minor comments but LGTM.
@ronkorving @3rd-Eden So you both prefer to use Object.keys
instead of for ... in
?
I found another problem when I use Object.keys()
:
If the object is undefined
, for(var i in undefined)
will be ran as an empty object bug Object.keys(undefined)
will throw an error!
This error makes mocha
failed:
TypeError: Cannot convert undefined or null to object
at Function.keys (native)
at Object.merge (/Users/XadillaX/code/node/node-memcached/lib/utils.js:113:10)
at new Client (/Users/XadillaX/code/node/node-memcached/lib/memcached.js:58:9)
at Context.<anonymous> (/Users/XadillaX/code/node/node-memcached/test/memcached-touch.test.js:21:21)
at Test.Runnable.run (/usr/local/lib/node_modules/mocha/lib/runnable.js:217:15)
at Runner.runTest (/usr/local/lib/node_modules/mocha/lib/runner.js:373:10)
at /usr/local/lib/node_modules/mocha/lib/runner.js:451:12
at next (/usr/local/lib/node_modules/mocha/lib/runner.js:298:14)
at /usr/local/lib/node_modules/mocha/lib/runner.js:308:7
at next (/usr/local/lib/node_modules/mocha/lib/runner.js:246:23)
at Immediate._onImmediate (/usr/local/lib/node_modules/mocha/lib/runner.js:275:5)
at processImmediate [as _immediateCallback] (timers.js:321:17)
So I think we still should use for ... in
, or we should make sure object is not null at first: if(object !== undefined && object !== null)
.
@3rd-Eden @ronkorving
Correct, Object.keys breaks on non-objects. A simple truthy check would address that, right? That's my preferred style. But I suggest you go with whatever style @3rd-Eden prefers.
if (obj) {
var keys = Object.keys(obj);
}
But doing a for in
on an undefined
is just as bad as doing an Object.keys
on undefined. I would argue that a thrown error would be better as you're simply not receiving what you're expecting as the only proper use-case for a for in
loop would be to iterate over an object.
If the code fails with Object.key
there is a bigger problem with the code somewhere as it shouldn't even iterate on it.