Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement: expose isIndex, isLength and complimentary toLength #1590

Closed
Xotic750 opened this issue Oct 30, 2015 · 10 comments
Closed

Enhancement: expose isIndex, isLength and complimentary toLength #1590

Xotic750 opened this issue Oct 30, 2015 · 10 comments

Comments

@Xotic750
Copy link

Change reIsUint to pick up number string literals beginning with 0.
Change isLength, add length argument, use other lodash methods
Add toLength as it doesn't exist.
These are all useful methods that I find myself using and lodash mostly has them internally.

var MAX_SAFE_INTEGER = Number.MAX_SAFE_INTEGER || Math.pow(2, 53) - 1;

/** Used to detect unsigned integer values. */
var reIsUint = /^(\d|[1-9]\d+)$/;

/**
 * Checks if `value` is a valid array-like index.
 *
 * @param {*} value The value to check.
 * @param {number} [length=MAX_SAFE_INTEGER] The upper bounds of a valid index.
 * @returns {boolean} Returns `true` if `value` is a valid index, else `false`.
 */
function isIndex(value, length) {
  value = (typeof value == 'number' || reIsUint.test(value)) ? +value : -1;
  length = length == null ? MAX_SAFE_INTEGER : length;
  return value > -1 && value % 1 == 0 && value < length;
}

/**
 * Checks if `value` is a valid array-like length.
 *
 * **Note:** This function is based on [`ToLength`](http://ecma-international.org/ecma-262/6.0/#sec-tolength).
 *
 * @param {*} value The value to check.
 * @param {number} [length=MAX_SAFE_INTEGER] The upper bounds of a valid length.
 * @returns {boolean} Returns `true` if `value` is a valid length, else `false`.
 */
function isLength(value, length) {
  value = (typeof value == 'number' || reIsUint.test(value)) ? +value : -1;
  length = length == null ? MAX_SAFE_INTEGER : length;
  return value > -1 && value % 1 == 0 && value <= length;
}

/**
 * The abstract operation ToLength converts its argument to an integer
 * suitable for use as the length of an array-like object.
 *
 * **Note:** This function is based on [`ToLength`](http://ecma-international.org/ecma-262/6.0/#sec-tolength).
 *
 * @param {*} value The object to be converted to a length.
 * @param {number} [length=MAX_SAFE_INTEGER] The upper bounds of a valid length.
 * @return {number} If len <= 0 then 0 else if len is +INFINITY then 2^53-1
 *                  else min(len, 2^53-1).
 */
function toLength(value, length) {
  length = length == null ? MAX_SAFE_INTEGER : length;
  return _.clamp(_.toInteger(value), 0, length);
}
@jdalton
Copy link
Member

jdalton commented Oct 30, 2015

I want to hold off on exposing isIndex and isLength because I don't think they're useful outside of internals opting for using _.isArrayLike instead. Do you have a use case in mind?

@Xotic750
Copy link
Author

One example (isIndex) is when patching buggy defineProperty/defineProperties, I've also used it for a hasProperty method, where native in isn't enough (buggy). Possibly I could do the same with _.isArrayLike, but would need to check. isLength I've used several times, again _.isArrayLike may work instead. So these would just be nice to haves I guess. toLength I use frequently, for all sorts of methods where I want the interface to have an ES feel about it. So this I would definitely like out of all of them. Pretty much every occasion where you may consider value >>> 0, but toLength is much nicer I think. You still may want to consider a regex change though, even internally.

@jdalton
Copy link
Member

jdalton commented Oct 30, 2015

Pretty much every occasion where you may consider value >>> 0, but toLength is much nicer I think

Ah good point. I'll accept a PR for exposing isLength and adding a toLength.

You still may want to consider a regex change though, even internally.

I want to keep isIndex internal but hardening the reIsUint to avoid letting '0001' slip through is a good idea so I'll accept a different PR for that too.

@Xotic750
Copy link
Author

Sounds reasonable. Now, the question is can I work git to have multiple branches for concurrent PRs. I've only worked with having a master and that is occupied with the toNumber PR. Or maybe I'll just wait a bit. :)

@jdalton
Copy link
Member

jdalton commented Oct 30, 2015

You can create a branch as:

cd lodash
git checkout master
git branch isLength
git checkout isLength
# do stuff
git push origin isLength
git checkout master
git branch reIsUnit
git checkout reIsUnit
# do stuff
git push origin reIsUnit
git checkout master

@Xotic750
Copy link
Author

I'll give it a try and hopefully not end up in too much of a mess.

@Xotic750
Copy link
Author

I'm assuming that you want isLength to remain as is and just be exposed (if I am understanding correctly), as my suggested change would cause a difference in results. As currently it must specifically be of type number rather than something that coerces to a valid number of isLength?

@jdalton
Copy link
Member

jdalton commented Oct 30, 2015

Exposed methods get a bit more robust to handle more edge cases. To start out the PR could just expose them as is and I'll comment tweak once the PR is opened.

@Xotic750
Copy link
Author

Ok, I will give that a try and see how it goes with git. :)

Xotic750 pushed a commit to Xotic750/lodash that referenced this issue Oct 30, 2015
Xotic750 pushed a commit to Xotic750/lodash that referenced this issue Oct 30, 2015
Xotic750 pushed a commit to Xotic750/lodash that referenced this issue Oct 30, 2015
@lock
Copy link

lock bot commented Jan 19, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

2 participants