added a few more test cases #964

Merged
merged 1 commit into from Feb 12, 2013

Projects

None yet

2 participants

Contributor
clottes commented Feb 6, 2013

I'd like to add a few more simple test cases, just for robustness. (they've helped me optimize a few functions to be faster without causing problems)

@jashkenas jashkenas merged commit e0dd1f7 into jashkenas:master Feb 12, 2013

1 check passed

default The Travis build passed
Details
Owner

(they've helped me optimize a few functions to be faster without causing problems)

Feel like sharing?

Contributor
clottes commented Feb 13, 2013

Sure, I'm not sure if you are in favor of simplicity, readability, or speed. Personally all are important to me.
I'll definitely share what I've done, especially if I can contribute! Some of them are small speed improvements, such as the _.isString _.isFunction and _.isNumber.

A few questions:

  1. What do you think of providing Implementations of the ECMAScript 5 native functions if not supported in browser adding to the prototype objects if not already there? This would ensure that all those functions actually exist and remove the need to check for their existence. (though if you feel that shouldn't be in the scope of this library, that makes sense), but to me it's where they belong since the functionality is so entwined, and what's wrong with providing those functions if not there in all browsers, I can't think of a reason.

  2. Should I just do more pull requests for ideas I have?

  3. calling Functions seems like an expensive operation, so in what I've changed in my branch, I've pulled some of the nested function calls out, say where the iterator inside of each simply does another function.call (I've replaced with a for or while loop, etc... I'll show you what I mean shortly. I've tried to pull it out where there are LARGE speed improvements without going nuts and keeping it still very simple, I'll be happy to share, I'll send a few your way in the future. (I'm at work now so don't have the time immediately)

    There are a few things I've seen/fixed which I'll share shortly when I get a chance, such as more problems with some functions handling NodeLists in IE<9.

Owner
  1. Nope -- that's what es5-shim is good for. Underscore tries to stay away from that.
  2. Yep, if you think it's something worth merging in.
  3. We generally prefer delegating to other core Underscore functions when possible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment