No. The problem as stated on #4571 was that the `{radius: 30, length: 0}`
object contained a length property, and that Arguments objects weren't
being recognized as array-like. This caused the `iterator` not to be called
in `forEach`, consequently not transferring the properties of the source
object to the destination object. The angular `forEach` uses `isArrayLike`
detection to determine what implementation of `forEach` to call: native,
for loop, or object property iteration.

My defense of the code is below the following request for help.

I am concerned about `hasOwnProperty` in IE8. I have a polyfill designed,
listed below, that I think will work correctly, but I cannot seem to get
the modern IE testing vms to load in VirtualBox. I may have to switch to
the windows side of my box and test it out.

```js
if(!Object.prototype.hasOwnProperty) {
  //polyfill for hasOwnProperty
  Object.prototype.hasOwnProperty = function(propertyName) {
    return ((isDefined(this.prototype)) ? isDefined(this[propertyName]) &&
!isDefined(this.prototype[propertyName]) : isDefined(this[propertyName]) &&
!isDefined(this.__proto__[propertyName]))
  };
}
```

Anyone that has access to a win8 browser, could you plug that into the
developer console and tell me if it works? The alternative is an iterative
approach using an object.keys polyfill that we should probably avoid.
`isDefined` is from the Angular.js source, you may copy and paste it into
the console before the above code or load angular.js into a test harness
testable on ie8.

Defense

As for the long expressions, an early exit optimized version of that code
only achieved a 14% increase over the pr version relative to the master in
the performance tests linked above. According to 11.11 of the
http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-262.pdf spec
Section 11.11, logical or short-circuits/early-exits when it encounters a
true value. The return expression and the early exit expression are thus
functionally equivalent, and the performance tests suggest this to be true.
14% difference in speed is not a significant performance degradation.

The benefits of reference locality should be couched in the cause of
correctness, clarity of intent, and avoiding double evaluation. The
variables are local to the function. The expression is correct. It reads,
in plain English, return the result of "the object exists and is not the
window object and one of the following; a node list, an arguments object, a
string, or an array or some other object with numerical, ordered keys that
is not in the previous list." The intent is to show that the expression is
enumerable by number key via detecting what it is. The need to store
evaluated expressions as local variables arises because the
`isSomeOtherObject` assertion requires the negation of is_array, is_string,
etc. Evaluation in the head and referencing in the return avoids
unnecessary code duplication in the return. All the tests are necessary to
exhaustively identify the property of iteration type (ordered or not)
within the scope of the Angular semantics, and additional or expression
nesting, avoiding the local storage, would make the intent less clear.

In addition, the location of the return statement relative to the variables
referenced within it on a given editor with a given font-size on a given
monitor at a given resolution while remaining within the coding guidelines
of the project make the judgement of where the screen-break occurs
subjective and difficult to judge.

If statements returning booleans create opportunites for confusion. Since
an if statement takes a boolean, returning a boolean as the result of an if
statement is doing double work, as well as providing opportunity for the
boolean evaluated in the if clause to lie --

```js
if(true) return false;
```

which can cause confusion for developers reading the code. In addition,
adding business conditionals to a deeply nested if statement is difficult
due to additive else if clauses. Assigning the new business conditional to
a local variable or collapsing a complicated boolean business expression
into an assignment makes for easier modification and legibility of the code
when modification becomes necessary.


On Sun, Nov 10, 2013 at 5:33 AM, Caitlin Potter <notifications@github.com>wrote:

> In test/AngularSpec.js:
>
> > @@ -159,6 +159,13 @@ describe('angular', function() {
> >        expect(hashKey(dst)).not.toEqual(hashKey(src));
> >      });
> >
> > +    it('should return the object with the cloned properties, even if empty', function(){
> > +      var options;
> > +      options = {};
> > +      options = extend(options, {radius: 30, length: 0});
> > +      expect(options.hasOwnProperty('radius')).toBe(true);
>
> I don't totally understand this test, are we saying that extend() was
> broken because the extend object looks array-like to angular?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/angular/angular.js/pull/4855/files#r7548439>
> .
>